diff --git a/openhcl/underhill_core/src/nvme_manager/device.rs b/openhcl/underhill_core/src/nvme_manager/device.rs index 3fb4892297..6109eaafd6 100644 --- a/openhcl/underhill_core/src/nvme_manager/device.rs +++ b/openhcl/underhill_core/src/nvme_manager/device.rs @@ -82,6 +82,7 @@ impl CreateNvmeDriver for VfioNvmeDriverSpawner { pci_id: &str, vp_count: u32, save_restore_supported: bool, + fused_keepalive_device: bool, mut saved_state: Option<&NvmeDriverSavedState>, ) -> Result, NvmeSpawnerError> { // Gracefully tear down old state & reset device if a saved state is @@ -175,6 +176,7 @@ impl CreateNvmeDriver for VfioNvmeDriverSpawner { vfio_device, saved_state, self.is_isolated, + fused_keepalive_device, ) .instrument(tracing::info_span!("nvme_driver_restore")) .await @@ -186,6 +188,7 @@ impl CreateNvmeDriver for VfioNvmeDriverSpawner { vp_count, self.nvme_always_flr, self.is_isolated, + fused_keepalive_device, dma_clients, ) .await? @@ -224,6 +227,7 @@ impl VfioNvmeDriverSpawner { vp_count: u32, nvme_always_flr: bool, is_isolated: bool, + fused_keepalive_device: bool, dma_clients: VfioDmaClients, ) -> Result, NvmeSpawnerError> { let mut last_err = None; @@ -243,6 +247,7 @@ impl VfioNvmeDriverSpawner { pci_id, vp_count, is_isolated, + fused_keepalive_device, dma_clients.clone(), ) .await @@ -280,6 +285,7 @@ impl VfioNvmeDriverSpawner { pci_id: &str, vp_count: u32, is_isolated: bool, + fused_keepalive_device: bool, dma_clients: VfioDmaClients, ) -> Result, NvmeSpawnerError> { let device = VfioDevice::new(driver_source, pci_id, dma_clients) @@ -291,10 +297,20 @@ impl VfioNvmeDriverSpawner { // TODO: For now, any isolation means use bounce buffering. This // needs to change when we have nvme devices that support DMA to // confidential memory. - nvme_driver::NvmeDriver::new(driver_source, vp_count, device, is_isolated) - .instrument(tracing::info_span!("nvme_driver_new", pci_id)) - .await - .map_err(NvmeSpawnerError::DeviceInitFailed) + nvme_driver::NvmeDriver::new( + driver_source, + vp_count, + device, + is_isolated, + fused_keepalive_device, + ) + .instrument(tracing::info_span!( + "nvme_driver_new", + pci_id, + fused_keepalive_device + )) + .await + .map_err(NvmeSpawnerError::DeviceInitFailed) } fn try_update_reset_method(pci_id: &str, method: PciDeviceResetMethod, label: &str) { @@ -364,6 +380,7 @@ impl NvmeDriverManager { pci_id: &str, vp_count: u32, save_restore_supported: bool, + fused_keepalive_device: bool, device: Option>, nvme_driver_spawner: Arc, ) -> anyhow::Result { @@ -375,6 +392,7 @@ impl NvmeDriverManager { pci_id: pci_id.into(), vp_count, save_restore_supported, + fused_keepalive_device, driver: device, nvme_driver_spawner, }; @@ -494,6 +512,9 @@ struct NvmeDriverManagerWorker { vp_count: u32, /// Whether the running environment (specifically the VTL2 memory layout) allows save/restore. save_restore_supported: bool, + /// WORKAROUND: a subset of devices require "fused keepalive". This flag signals to the NVMe + /// driver that this device's admin queues may be unusable after a servicing event + fused_keepalive_device: bool, #[inspect(skip)] nvme_driver_spawner: Arc, driver: Option>, @@ -528,6 +549,7 @@ impl NvmeDriverManagerWorker { &self.pci_id, self.vp_count, self.save_restore_supported, + self.fused_keepalive_device, None, ) .await?; diff --git a/openhcl/underhill_core/src/nvme_manager/manager.rs b/openhcl/underhill_core/src/nvme_manager/manager.rs index 9ca8c6fb25..3aed459a19 100644 --- a/openhcl/underhill_core/src/nvme_manager/manager.rs +++ b/openhcl/underhill_core/src/nvme_manager/manager.rs @@ -5,6 +5,7 @@ use crate::nvme_manager::CreateNvmeDriver; use crate::nvme_manager::device::NvmeDriverManager; use crate::nvme_manager::device::NvmeDriverManagerClient; use crate::nvme_manager::device::NvmeDriverShutdownOptions; +use crate::nvme_manager::is_nvme_fused_keepalive_device; use crate::nvme_manager::save_restore::NvmeManagerSavedState; use crate::nvme_manager::save_restore::NvmeSavedDiskConfig; use crate::servicing::NvmeSavedState; @@ -14,6 +15,7 @@ use disk_backend::resolve::ResolveDiskParameters; use disk_backend::resolve::ResolvedDisk; use futures::StreamExt; use futures::future::join_all; +use futures::future::try_join_all; use inspect::Inspect; use mesh::MeshPayload; use mesh::rpc::Rpc; @@ -338,6 +340,8 @@ impl NvmeManagerWorker { // Note: `client` exists outside of the devices write lock. This is safe: // the mesh client will fail appropriately if shutdown comes in between inserting // this entry and the call to `load_driver()`. + let fused_keepalive_device = is_nvme_fused_keepalive_device(&pci_id); + let client = { let mut guard = context.devices.write(); @@ -360,6 +364,7 @@ impl NvmeManagerWorker { &pci_id, context.vp_count, context.save_restore_supported, + fused_keepalive_device, None, // No device yet, context.nvme_driver_spawner.clone(), )?; @@ -445,29 +450,39 @@ impl NvmeManagerWorker { saved_state: &NvmeManagerSavedState, save_restore_supported: bool, ) -> anyhow::Result<()> { - let mut restored_devices: HashMap = HashMap::new(); - - for disk in &saved_state.nvme_disks { + let context = &self.context; + let created = try_join_all(saved_state.nvme_disks.iter().map(|disk| { let pci_id = disk.pci_id.clone(); - let nvme_driver = self - .context - .nvme_driver_spawner - .create_driver( - &self.context.driver_source, - &pci_id, - saved_state.cpu_count, - save_restore_supported, - Some(&disk.driver_state), - ) - .await?; + async move { + let fused_keepalive_device = is_nvme_fused_keepalive_device(&pci_id); + + let nvme_driver = context + .nvme_driver_spawner + .create_driver( + &context.driver_source, + &pci_id, + saved_state.cpu_count, + save_restore_supported, + fused_keepalive_device, + Some(&disk.driver_state), + ) + .await?; + + anyhow::Ok((pci_id, fused_keepalive_device, nvme_driver)) + } + })) + .await?; + let mut restored_devices: HashMap = HashMap::new(); + for (pci_id, fused_keepalive_device, nvme_driver) in created { restored_devices.insert( - disk.pci_id.clone(), + pci_id.clone(), NvmeDriverManager::new( &self.context.driver_source, &pci_id, self.context.vp_count, true, // save_restore_supported is always `true` when restoring. + fused_keepalive_device, Some(nvme_driver), self.context.nvme_driver_spawner.clone(), )?, @@ -748,6 +763,7 @@ mod tests { pci_id: &str, _vp_count: u32, _save_restore_supported: bool, + _fused_keepalive_device: bool, _saved_state: Option<&NvmeDriverSavedState>, ) -> Result, NvmeSpawnerError> { if self.fail_create.load(Ordering::SeqCst) { @@ -1068,9 +1084,16 @@ mod tests { )); // Create a driver manager - let driver_manager = - NvmeDriverManager::new(&driver_source, "0000:00:04.0", 4, false, None, spawner) - .unwrap(); + let driver_manager = NvmeDriverManager::new( + &driver_source, + "0000:00:04.0", + 4, + false, + false, + None, + spawner, + ) + .unwrap(); let client = driver_manager.client().clone(); @@ -1153,7 +1176,8 @@ mod tests { &driver_source, "0000:00:05.0", 4, - true, // save_restore_supported + true, // save_restore_supported + false, // fused_keepalive_device None, spawner, ) diff --git a/openhcl/underhill_core/src/nvme_manager/mod.rs b/openhcl/underhill_core/src/nvme_manager/mod.rs index cc8977de45..2854208704 100644 --- a/openhcl/underhill_core/src/nvme_manager/mod.rs +++ b/openhcl/underhill_core/src/nvme_manager/mod.rs @@ -67,6 +67,14 @@ pub struct NamespaceError { source: NvmeSpawnerError, } +/// PCI vendor ID, as it appears in the sysfs `vendor` file (e.g. `0x0100`), +/// for NVMe devices that require fused keepalive device mode. +const FUSED_DEVICE_VENDOR_ID: &str = "0x1414"; + +/// PCI device ID, as it appears in the sysfs `device` file (e.g. `0x0100`), +/// for NVMe devices that require fused keepalive device mode. +const FUSED_DEVICE_DEVICE_ID: &str = "0xb111"; + #[derive(Debug, Error)] pub enum NvmeSpawnerError { #[error("failed to initialize vfio device")] @@ -108,6 +116,48 @@ pub trait CreateNvmeDriver: Inspect + Send + Sync { pci_id: &str, vp_count: u32, save_restore_supported: bool, + fused_keepalive_device: bool, saved_state: Option<&nvme_driver::save_restore::NvmeDriverSavedState>, ) -> Result, NvmeSpawnerError>; } + +/// Returns whether the given PCI device requires fused keepalive device mode +pub(crate) fn is_nvme_fused_keepalive_device(pci_id: &str) -> bool { + match read_pci_vendor_device_ids(pci_id) { + Ok((vendor_id, device_id)) => { + vendor_id == FUSED_DEVICE_VENDOR_ID && device_id == FUSED_DEVICE_DEVICE_ID + } + Err(err) => { + tracing::warn!( + pci_id = %pci_id, + error = err.as_ref() as &dyn std::error::Error, + "failed to read PCI vendor/device IDs; treating device as a fused keepalive device" + ); + true + } + } +} + +/// Reads the sysfs `vendor` and `device` files for the given PCI device, +/// returning the trimmed contents (e.g. `"0x0100"`). +/// +/// Callers should invoke this once per device and cache the result, since +/// the values do not change for the lifetime of the device. +fn read_pci_vendor_device_ids(pci_id: &str) -> anyhow::Result<(String, String)> { + let devpath = std::path::Path::new("/sys/bus/pci/devices").join(pci_id); + let vendor = fs_err::read_to_string(devpath.join("vendor"))? + .trim_end() + .to_owned(); + let device = fs_err::read_to_string(devpath.join("device"))? + .trim_end() + .to_owned(); + + tracing::info!( + pci_id = %pci_id, + vendor = %vendor, + device = %device, + "read PCI vendor/device IDs" + ); + + Ok((vendor, device)) +} diff --git a/openhcl/underhill_core/src/nvme_manager/save_restore_helpers.rs b/openhcl/underhill_core/src/nvme_manager/save_restore_helpers.rs index ba6c3b2849..b846281818 100644 --- a/openhcl/underhill_core/src/nvme_manager/save_restore_helpers.rs +++ b/openhcl/underhill_core/src/nvme_manager/save_restore_helpers.rs @@ -174,6 +174,7 @@ mod tests { IoQueueSavedState { cpu, iv: qid as u32, + unmapped: false, queue_data: QueuePairSavedState { mem_len: 0, base_pfn: 0, diff --git a/vm/devices/storage/disk_nvme/nvme_driver/fuzz/fuzz_nvme_driver.rs b/vm/devices/storage/disk_nvme/nvme_driver/fuzz/fuzz_nvme_driver.rs index d862d6d17a..f324cfff8f 100644 --- a/vm/devices/storage/disk_nvme/nvme_driver/fuzz/fuzz_nvme_driver.rs +++ b/vm/devices/storage/disk_nvme/nvme_driver/fuzz/fuzz_nvme_driver.rs @@ -64,7 +64,15 @@ impl FuzzNvmeDriver { .unwrap(); let device = FuzzEmulatedDevice::new(nvme, msi_set, mem.dma_client()); - let mut nvme_driver = NvmeDriver::new(&driver_source, cpu_count, device, false).await?; // TODO: [use-arbitrary-input] + let fused_keepalive_device: bool = arbitrary_data::()?; + let mut nvme_driver = NvmeDriver::new( + &driver_source, + cpu_count, + device, + false, + fused_keepalive_device, + ) + .await?; // TODO: [use-arbitrary-input] let namespace = nvme_driver.namespace(1).await?; // TODO: [use-arbitrary-input] Ok(Self { diff --git a/vm/devices/storage/disk_nvme/nvme_driver/src/driver.rs b/vm/devices/storage/disk_nvme/nvme_driver/src/driver.rs index 3834ab583e..2d5e162929 100644 --- a/vm/devices/storage/disk_nvme/nvme_driver/src/driver.rs +++ b/vm/devices/storage/disk_nvme/nvme_driver/src/driver.rs @@ -84,6 +84,10 @@ pub struct NvmeDriver { /// Keeps the controller connected (CC.EN==1) while servicing. nvme_keepalive: bool, bounce_buffer: bool, + /// WORKAROUND: a subset of devices require "fused keepalive". When this flag is + /// set, the driver must be prepared to continue normally if admin queues become + /// unusable after a servicing event + fused_keepalive_device: bool, } /// A container that can hold either a weak or strong reference to a value. @@ -158,7 +162,7 @@ struct WorkerState { max_io_queues: u16, qsize: u16, #[inspect(skip)] - async_event_task: Task<()>, + async_event_task: Option>, } /// An error restoring from saved state. @@ -196,6 +200,10 @@ struct IoQueue { queue: QueuePair, iv: u16, cpu: u32, + /// WORKAROUND: for fused keepalive devices, we eagerly initialize + /// IO queues. However, we continue to wait for an IO before mapping + /// the interrupt to a CPU + unmapped: bool, } impl IoQueue { @@ -204,6 +212,7 @@ impl IoQueue { cpu: self.cpu, iv: self.iv as u32, queue_data: self.queue.save().await?, + unmapped: self.unmapped, }) } @@ -221,6 +230,7 @@ impl IoQueue { cpu, iv, queue_data, + unmapped, } = saved_state; let queue = QueuePair::restore( spawner, @@ -232,12 +242,14 @@ impl IoQueue { bounce_buffer, NoOpAerHandler, drain_after_restore, + false, )?; Ok(Self { queue, iv: *iv as u16, cpu: *cpu, + unmapped: *unmapped, }) } } @@ -271,11 +283,18 @@ impl NvmeDriver { cpu_count: u32, device: D, bounce_buffer: bool, + fused_keepalive_device: bool, ) -> anyhow::Result { let pci_id = device.id().to_owned(); - let mut this = Self::new_disabled(driver_source, cpu_count, device, bounce_buffer) - .instrument(tracing::info_span!("nvme_new_disabled", pci_id)) - .await?; + let mut this = Self::new_disabled( + driver_source, + cpu_count, + device, + bounce_buffer, + fused_keepalive_device, + ) + .instrument(tracing::info_span!("nvme_new_disabled", pci_id)) + .await?; match this .enable(cpu_count as u16) .instrument(tracing::info_span!("nvme_enable", pci_id)) @@ -300,6 +319,7 @@ impl NvmeDriver { cpu_count: u32, mut device: D, bounce_buffer: bool, + fused_keepalive_device: bool, ) -> anyhow::Result { let driver = driver_source.simple(); let bar0 = Bar0( @@ -361,6 +381,7 @@ impl NvmeDriver { namespaces: Default::default(), nvme_keepalive: false, bounce_buffer, + fused_keepalive_device, }) } @@ -384,7 +405,13 @@ impl NvmeDriver { .map_interrupt(0, 0) .context("failed to map interrupt 0")?; - // Start the admin queue pair. + // Start the admin queue pair. For fused keepalive devices, disable AER + // since no admin commands may be issued after initialization. + let aer_handler = if self.fused_keepalive_device { + AdminAerHandler::new_disabled() + } else { + AdminAerHandler::new() + }; let admin = QueuePair::new( self.driver.clone(), worker.device.deref(), @@ -394,8 +421,9 @@ impl NvmeDriver { interrupt0, worker.registers.clone(), self.bounce_buffer, - AdminAerHandler::new(), + aer_handler, DrainAfterRestoreBuilder::new_no_drain(), + false, ) .context("failed to create admin queue pair")?; @@ -544,18 +572,23 @@ impl NvmeDriver { }; // Spawn a task to handle asynchronous events. - let async_event_task = self.driver.spawn("nvme_async_event", { - let admin = admin.issuer().clone(); - let rescan_notifiers = self.rescan_notifiers.clone(); - async move { - if let Err(err) = handle_asynchronous_events(&admin, rescan_notifiers).await { - tracing::error!( - error = err.as_ref() as &dyn std::error::Error, - "asynchronous event failure, not processing any more" - ); + // When fused_keepalive_device is set, no commands are issued on the admin queue after init. + let async_event_task = if !self.fused_keepalive_device { + Some(self.driver.spawn("nvme_async_event", { + let admin = admin.issuer().clone(); + let rescan_notifiers = self.rescan_notifiers.clone(); + async move { + if let Err(err) = handle_asynchronous_events(&admin, rescan_notifiers).await { + tracing::error!( + error = err.as_ref() as &dyn std::error::Error, + "asynchronous event failure, not processing any more" + ); + } } - } - }); + })) + } else { + None + }; let mut state = WorkerState { qsize, @@ -565,14 +598,49 @@ impl NvmeDriver { self.admin = Some(admin.issuer().clone()); - // Pre-create the IO queue 1 for CPU 0. The other queues will be created - // lazily. Numbering for I/O queues starts with 1 (0 is Admin). - let issuer = worker - .create_io_queue(&mut state, 0) - .await - .context("failed to create io queue 1")?; + if self.fused_keepalive_device { + // Pre-create IO queues for all CPUs, all targeting CPU 0 initially. + // Interrupts will be lazily re-mapped when a CPU first does IO. + let num_queues = max_io_queues.min(self.io_issuers.per_cpu.len() as u16); + tracing::info!( + num_queues, + pci_id = ?self.device_id, + "fused keepalive device mode: eagerly pre-creating all io queues" + ); + for i in 0..num_queues { + let issuer = worker + .create_io_queue(&mut state, 0) + .await + .with_context(|| { + format!( + "failed to create io queue {} for fused keepalive device {}", + i + 1, + self.device_id + ) + })?; + if i == 0 { + // First queue is assigned to CPU 0 immediately. + self.io_issuers.per_cpu[0].set(issuer).unwrap(); + } else { + // Mark remaining queues as unmapped — they'll be claimed lazily. + let io_queue = worker.io.last_mut().unwrap(); + io_queue.unmapped = true; + } + } + } else { + // Pre-create the IO queue 1 for CPU 0. The other queues will be created + // lazily. Numbering for I/O queues starts with 1 (0 is Admin). + tracing::info!( + pci_id = ?self.device_id, + "pre-creating io queue 1 for cpu 0; remaining queues created lazily" + ); + let issuer = worker + .create_io_queue(&mut state, 0) + .await + .context("failed to create io queue 1")?; - self.io_issuers.per_cpu[0].set(issuer).unwrap(); + self.io_issuers.per_cpu[0].set(issuer).unwrap(); + } task.insert(&self.driver, "nvme_worker", state); task.start(); Ok(()) @@ -599,7 +667,9 @@ impl NvmeDriver { task.stop().await; let (worker, state) = task.into_inner(); if let Some(state) = state { - state.async_event_task.cancel().await; + if let Some(aen_task) = state.async_event_task { + aen_task.cancel().await; + } } // Hold onto responses until the reset completes so that waiting IOs do // not think the memory is unaliased by the device. @@ -750,6 +820,7 @@ impl NvmeDriver { mut device: D, saved_state: &NvmeDriverSavedState, bounce_buffer: bool, + fused_keepalive_device: bool, ) -> anyhow::Result { let pci_id = device.id().to_owned(); let driver = driver_source.simple(); @@ -806,6 +877,7 @@ impl NvmeDriver { namespaces: Default::default(), nvme_keepalive: true, bounce_buffer, + fused_keepalive_device, }; let task = &mut this.task.as_mut().unwrap(); @@ -828,18 +900,33 @@ impl NvmeDriver { .admin .as_ref() .map(|a| { + let pending_commands_count = a.handler_data.pending_cmds.commands.len(); tracing::info!( id = a.qid, - pending_commands_count = a.handler_data.pending_cmds.commands.len(), + pending_commands_count, ?pci_id, "restoring admin queue", ); + if fused_keepalive_device && pending_commands_count > 0 { + panic!( + "fused keepalive device {pci_id} restored with a non-empty admin \ + queue ({pending_commands_count} pending commands); fused devices \ + must not issue admin commands after init" + ); + } // Restore memory block for admin queue pair. let mem_block = restored_memory .iter() .find(|mem| mem.len() == a.mem_len && a.base_pfn == mem.pfns()[0]) .expect("unable to find restored mem block") .to_owned(); + // For fused keepalive devices, disable AER on restore since no + // admin commands may be issued after the BAR may be remapped. + let aer_handler = if fused_keepalive_device { + AdminAerHandler::new_disabled() + } else { + AdminAerHandler::new() + }; QueuePair::restore( driver.clone(), interrupt0, @@ -848,8 +935,9 @@ impl NvmeDriver { &pci_id, a, bounce_buffer, - AdminAerHandler::new(), + aer_handler, DrainAfterRestoreBuilder::new_no_drain(), // admin queue doesn't need draining + fused_keepalive_device, ) .expect("failed to restore admin queue pair") }) @@ -858,21 +946,27 @@ impl NvmeDriver { let admin = worker.admin.insert(admin); // Spawn a task to handle asynchronous events. - let async_event_task = this.driver.spawn("nvme_async_event", { - let admin = admin.issuer().clone(); - let rescan_notifiers = this.rescan_notifiers.clone(); - async move { - if let Err(err) = handle_asynchronous_events(&admin, rescan_notifiers) - .instrument(tracing::info_span!("async_event_handler")) - .await - { - tracing::error!( - error = err.as_ref() as &dyn std::error::Error, - "asynchronous event failure, not processing any more" - ); + // When fused_keepalive_device is set, no commands are issued on the admin queue after init. + let async_event_task = if !fused_keepalive_device { + Some(this.driver.spawn("nvme_async_event", { + let admin = admin.issuer().clone(); + let rescan_notifiers = this.rescan_notifiers.clone(); + async move { + if let Err(err) = handle_asynchronous_events(&admin, rescan_notifiers) + .instrument(tracing::info_span!("async_event_handler")) + .await + { + tracing::error!( + error = err.as_ref() as &dyn std::error::Error, + "asynchronous event failure, not processing any more" + ); + } } - } - }); + })) + } else { + tracing::info!("fused keepalive device mode: skipping async event handler on restore"); + None + }; let state = WorkerState { qsize: saved_state.worker_data.qsize, @@ -961,7 +1055,7 @@ impl NvmeDriver { .filter(|q| { q.queue_data.qid == 1 || !q.queue_data.handler_data.pending_cmds.commands.is_empty() }) - .flat_map(|q| -> Result, anyhow::Error> { + .map(|q| -> Result, anyhow::Error> { let qid = q.queue_data.qid; let cpu = q.cpu; tracing::info!(qid, cpu, ?pci_id, "restoring queue"); @@ -1005,7 +1099,7 @@ impl NvmeDriver { this.io_issuers.per_cpu[q.cpu as usize].set(issuer).unwrap(); Ok(q) }) - .collect(); + .collect::>>()?; // (2) Create prototype entries for any queues that don't currently have outstanding commands. // They will be restored on demand later. @@ -1275,18 +1369,88 @@ impl DriverWorkerTask { self.io_issuers.per_cpu[cpu as usize] .set(issuer) - .expect("issuer already set for this cpu"); + .unwrap_or_else(|_| { + panic!("io issuer for device {pci_id} on cpu {cpu} was already set") + }); self.io.push(queue); Ok(()) } + fn fallback_io_issuer(&self, cpu: u32) -> (usize, IoIssuer) { + self.io_issuers.per_cpu[..cpu as usize] + .iter() + .enumerate() + .rev() + .find_map(|(i, issuer)| issuer.get().map(|issuer| (i, issuer.clone()))) + .unwrap_or_else(|| { + panic!( + "io issuer for device {:?} on cpu {} failed to fallback. there must be at least one io issuer for cpu 0", + self.device.id(), + cpu + ) + }) + } + async fn create_io_issuer(&mut self, state: &mut WorkerState, cpu: u32) { tracing::debug!(cpu, pci_id = ?self.device.id(), "issuer request"); if self.io_issuers.per_cpu[cpu as usize].get().is_some() { return; } + // In fused keepalive device mode, claim an unmapped queue from the pool + // and re-target its interrupt to the requesting CPU. + if let Some(idx) = self.io.iter().position(|q| q.unmapped) { + let iv = self.io[idx].iv; + tracing::debug!( + cpu, + iv, + pci_id = ?self.device.id(), + "fused mode: claiming unmapped queue" + ); + match self.device.map_interrupt(iv.into(), cpu) { + Ok(_interrupt) => { + let io_queue = &mut self.io[idx]; + io_queue.cpu = cpu; + io_queue.unmapped = false; + let issuer = IoIssuer { + issuer: io_queue.queue.issuer().clone(), + cpu, + }; + self.io_issuers.per_cpu[cpu as usize] + .set(issuer) + .unwrap_or_else(|_| { + panic!( + "io issuer for device {:?} on cpu {} was already set", + self.device.id(), + cpu + ) + }); + } + Err(err) => { + let (fallback_cpu, fallback) = self.fallback_io_issuer(cpu); + tracing::error!( + cpu, + iv, + fallback_cpu, + pci_id = ?self.device.id(), + error = err.as_ref() as &dyn std::error::Error, + "fused mode: failed to re-target interrupt, sharing an existing issuer" + ); + self.io_issuers.per_cpu[cpu as usize] + .set(fallback) + .unwrap_or_else(|_| { + panic!( + "io issuer for device {:?} on cpu {} was already set", + self.device.id(), + cpu + ) + }); + } + } + return; + } + if let Some(proto) = self.proto_io.remove(&cpu) { match self.restore_io_issuer(proto) { Ok(()) => return, @@ -1316,12 +1480,7 @@ impl DriverWorkerTask { Ok(issuer) => issuer, Err(err) => { // Find a fallback queue close in index to the failed queue. - let (fallback_cpu, fallback) = self.io_issuers.per_cpu[..cpu as usize] - .iter() - .enumerate() - .rev() - .find_map(|(i, issuer)| issuer.get().map(|issuer| (i, issuer))) - .expect("unable to find an io issuer for fallback"); + let (fallback_cpu, fallback) = self.fallback_io_issuer(cpu); // Log the error as informational only when there is a lack of // hardware resources from the device. @@ -1346,14 +1505,19 @@ impl DriverWorkerTask { } } - fallback.clone() + fallback } }; self.io_issuers.per_cpu[cpu as usize] .set(issuer) - .ok() - .unwrap(); + .unwrap_or_else(|_| { + panic!( + "io issuer for device {:?} on cpu {} was already set", + self.device.id(), + cpu + ) + }); // Lazily clear the drain-after-restore builder once draining is done, // to free the shared Arc resources. @@ -1414,6 +1578,7 @@ impl DriverWorkerTask { self.bounce_buffer, NoOpAerHandler, drain_after_restore, + false, ) .map_err(|err| DeviceError::IoQueuePairCreationFailure(err, qid))?; @@ -1425,7 +1590,12 @@ impl DriverWorkerTask { // Add the queue pair before aliasing its memory with the device so // that it can be torn down correctly on failure. - self.io.push(IoQueue { queue, iv, cpu }); + self.io.push(IoQueue { + queue, + iv, + cpu, + unmapped: false, + }); let io_queue = self.io.last_mut().unwrap(); let admin = self.admin.as_ref().unwrap().issuer().as_ref(); @@ -1677,6 +1847,9 @@ pub mod save_restore { pub iv: u32, #[mesh(3)] pub queue_data: QueuePairSavedState, + #[mesh(4)] + /// When `true`, the queue has not yet been affinitized to its cpu. + pub unmapped: bool, } /// Save/restore state for QueueHandler task. diff --git a/vm/devices/storage/disk_nvme/nvme_driver/src/queue_pair.rs b/vm/devices/storage/disk_nvme/nvme_driver/src/queue_pair.rs index a669a1e630..6c9f661a70 100644 --- a/vm/devices/storage/disk_nvme/nvme_driver/src/queue_pair.rs +++ b/vm/devices/storage/disk_nvme/nvme_driver/src/queue_pair.rs @@ -92,11 +92,12 @@ impl PendingCommands { const MAX_CIDS: usize = 1 << Self::CID_KEY_BITS; const CID_SEQ_OFFSET: Wrapping = Wrapping(1 << Self::CID_KEY_BITS); - fn new(qid: u16) -> Self { + fn new(qid: u16, device_id: String) -> Self { Self { commands: Slab::new(), next_cid_high_bits: Wrapping(0), qid, + device_id, } } @@ -130,7 +131,12 @@ impl PendingCommands { let command = self .commands .try_remove((cid & Self::CID_KEY_MASK) as usize) - .unwrap_or_else(|| panic!("completion for unknown cid: qid={}, cid={}", self.qid, cid)); + .unwrap_or_else(|| { + panic!( + "completion for unknown cid {cid} on qid {} for device {}", + self.qid, self.device_id + ) + }); assert_eq!( command.command.cdw0.cid(), cid, @@ -159,7 +165,11 @@ impl PendingCommands { } /// Restore pending commands from the saved state. - pub fn restore(saved_state: &PendingCommandsSavedState, qid: u16) -> anyhow::Result { + pub fn restore( + saved_state: &PendingCommandsSavedState, + qid: u16, + device_id: String, + ) -> anyhow::Result { let PendingCommandsSavedState { commands, next_cid_high_bits, @@ -185,6 +195,7 @@ impl PendingCommands { .collect::>(), next_cid_high_bits: Wrapping(*next_cid_high_bits), qid, + device_id, }) } } @@ -390,6 +401,7 @@ impl QueuePair { bounce_buffer: bool, aer_handler: A, drain_after_restore: DrainAfterRestore, + commands_forbidden: bool, ) -> anyhow::Result { // FUTURE: Consider splitting this into several allocations, rather than // allocating the sum total together. This can increase the likelihood @@ -425,6 +437,7 @@ impl QueuePair { bounce_buffer, aer_handler, drain_after_restore, + commands_forbidden, ) } @@ -441,6 +454,7 @@ impl QueuePair { bounce_buffer: bool, aer_handler: A, drain_after_restore: DrainAfterRestore, + commands_forbidden: bool, ) -> anyhow::Result { // MemoryBlock is either allocated or restored prior calling here. let sq_mem_block = mem.subblock(0, SQ_SIZE); @@ -499,18 +513,20 @@ impl QueuePair { device_id, qid, drain_after_restore, + commands_forbidden, )?, None => { // Create a new one. QueueHandler { sq: SubmissionQueue::new(qid, sq_entries, sq_mem_block), cq: CompletionQueue::new(qid, cq_entries, cq_mem_block), - commands: PendingCommands::new(qid), + commands: PendingCommands::new(qid, device_id.into()), stats: Default::default(), drain_after_restore, aer_handler, device_id: device_id.into(), qid, + commands_forbidden, } } }; @@ -628,6 +644,7 @@ impl QueuePair { bounce_buffer: bool, aer_handler: A, drain_after_restore: DrainAfterRestore, + commands_forbidden: bool, ) -> anyhow::Result { let QueuePairSavedState { mem_len: _, // Used to restore DMA buffer before calling this. @@ -651,6 +668,7 @@ impl QueuePair { bounce_buffer, aer_handler, drain_after_restore, + commands_forbidden, ) } } @@ -932,6 +950,8 @@ struct PendingCommands { #[inspect(hex)] next_cid_high_bits: Wrapping, qid: u16, + #[inspect(skip)] + device_id: String, } #[derive(Inspect)] @@ -994,6 +1014,9 @@ pub struct AdminAerHandler { await_aen_cid: Option, send_aen: Option>>, // Channel to return AENs on. failed_status: Option, // If the failed state is reached, it will stop looping until save/restore. + /// When true, no AER commands will be issued. Used for fused keepalive + /// devices where admin commands must never be issued after initialization. + disabled: bool, } impl AdminAerHandler { @@ -1003,6 +1026,19 @@ impl AdminAerHandler { await_aen_cid: None, send_aen: None, failed_status: None, + disabled: false, + } + } + + /// Creates a handler that never issues AER commands. Used for fused + /// keepalive devices where the admin queue must remain idle after init. + pub fn new_disabled() -> Self { + Self { + last_aen: None, + await_aen_cid: None, + send_aen: None, + failed_status: None, + disabled: true, } } } @@ -1049,7 +1085,7 @@ impl AerHandler for AdminAerHandler { } fn poll_send_aer(&self) -> bool { - self.await_aen_cid.is_none() && self.failed_status.is_none() + !self.disabled && self.await_aen_cid.is_none() && self.failed_status.is_none() } fn update_awaiting_cid(&mut self, cid: u16) { @@ -1111,6 +1147,7 @@ struct QueueHandler { aer_handler: A, device_id: String, qid: u16, + commands_forbidden: bool, } #[derive(Inspect, Default)] @@ -1220,6 +1257,12 @@ impl QueueHandler { }, Event::Command(cmd) => match cmd { Cmd::Command(rpc) => { + if self.commands_forbidden { + panic!( + "attempted to submit a command to admin queue {} for device {} after restore; the admin queue must remain idle for fused keepalive devices", + self.qid, self.device_id + ); + } let (mut command, respond) = rpc.split(); self.commands.insert(&mut command, respond); self.sq.write(command).unwrap(); @@ -1278,6 +1321,7 @@ impl QueueHandler { device_id: &str, qid: u16, drain_after_restore: DrainAfterRestore, + commands_forbidden: bool, ) -> anyhow::Result { let QueueHandlerSavedState { sq_state, @@ -1291,7 +1335,7 @@ impl QueueHandler { Ok(Self { sq: SubmissionQueue::restore(sq_mem_block, sq_state)?, cq: CompletionQueue::restore(cq_mem_block, cq_state)?, - commands: PendingCommands::restore(pending_cmds, sq_state.sqid)?, + commands: PendingCommands::restore(pending_cmds, sq_state.sqid, device_id.into())?, stats: Default::default(), // Only drain pending commands for I/O queues. // Admin queue is expected to have pending Async Event requests. @@ -1299,6 +1343,7 @@ impl QueueHandler { aer_handler, device_id: device_id.into(), qid, + commands_forbidden, }) } } diff --git a/vm/devices/storage/disk_nvme/nvme_driver/src/tests.rs b/vm/devices/storage/disk_nvme/nvme_driver/src/tests.rs index 0a1578eb75..34af34fcef 100644 --- a/vm/devices/storage/disk_nvme/nvme_driver/src/tests.rs +++ b/vm/devices/storage/disk_nvme/nvme_driver/src/tests.rs @@ -241,7 +241,7 @@ async fn test_nvme_ioqueue_max_mqes(driver: DefaultDriver) { let cap: Cap = Cap::new().with_mqes_z(max_u16); device.set_mock_response_u64(Some((0, cap.into()))); - let driver = NvmeDriver::new(&driver_source, CPU_COUNT, device, false).await; + let driver = NvmeDriver::new(&driver_source, CPU_COUNT, device, false, false).await; assert!(driver.is_ok()); } @@ -276,7 +276,7 @@ async fn test_nvme_ioqueue_invalid_mqes(driver: DefaultDriver) { // Setup mock response at offset 0 let cap: Cap = Cap::new().with_mqes_z(0); device.set_mock_response_u64(Some((0, cap.into()))); - let driver = NvmeDriver::new(&driver_source, CPU_COUNT, device, false).await; + let driver = NvmeDriver::new(&driver_source, CPU_COUNT, device, false, false).await; assert!(driver.is_err()); } @@ -335,12 +335,12 @@ async fn test_nvme_driver(driver: DefaultDriver, config: NvmeTestConfig) { if fail_at_driver_create { fail_alloc.store(true, Ordering::SeqCst); - let driver_result = NvmeDriver::new(&driver_source, CPU_COUNT, device, false).await; + let driver_result = NvmeDriver::new(&driver_source, CPU_COUNT, device, false, false).await; assert!(driver_result.is_err()); return; } - let mut driver = NvmeDriver::new(&driver_source, CPU_COUNT, device, false) + let mut driver = NvmeDriver::new(&driver_source, CPU_COUNT, device, false, false) .await .unwrap(); let namespace = driver.namespace(1).await.unwrap(); @@ -483,7 +483,7 @@ async fn test_nvme_fault_injection(driver: DefaultDriver, fault_configuration: F .await .unwrap(); let device = NvmeTestEmulatedDevice::new(nvme, msi_set, dma_client.clone()); - let mut driver = NvmeDriver::new(&driver_source, CPU_COUNT, device, false) + let mut driver = NvmeDriver::new(&driver_source, CPU_COUNT, device, false, false) .await .unwrap(); let namespace = driver.namespace(1).await.unwrap(); diff --git a/vm/devices/storage/storage_tests/tests/scsidvd_nvme.rs b/vm/devices/storage/storage_tests/tests/scsidvd_nvme.rs index 2e468f50b5..9f76a09ba6 100644 --- a/vm/devices/storage/storage_tests/tests/scsidvd_nvme.rs +++ b/vm/devices/storage/storage_tests/tests/scsidvd_nvme.rs @@ -80,7 +80,7 @@ impl ScsiDvdNvmeTest { .unwrap(); let device = EmulatedDevice::new(nvme, msi_set, dma_client.clone()); - let mut nvme_driver = NvmeDriver::new(&driver_source, CPU_COUNT, device, false) + let mut nvme_driver = NvmeDriver::new(&driver_source, CPU_COUNT, device, false, false) .await .unwrap(); let namespace = nvme_driver.namespace(1).await.unwrap();