[DNM] nvme: fused keepalive#3811
Open
babayet2 wants to merge 5 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Implements “fused keepalive” handling for a specific NVMe device class (VendorID 0x1414, DeviceID 0xb111) by ensuring the NVMe driver avoids issuing admin commands after initialization (notably disabling AER handling) and by eagerly initializing I/O queues to reduce post-servicing admin-queue dependence. This is integrated through Underhill’s NVMe manager by detecting the device and passing a fused_keepalive_device flag down into the NVMe driver, alongside updates to tests/fuzz harnesses for the new driver constructor signature.
Changes:
- Add a
fused_keepalive_devicemode toNvmeDriverthat disables AER/async event handling and eagerly creates IO queues. - Update Underhill NVMe manager/spawner interfaces to detect fused devices and propagate the fused-mode flag into driver creation/restore paths.
- Update unit tests, storage tests, fuzz harness, and servicing tests to reflect the new behavior and updated driver constructor.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| vmm_tests/vmm_tests/tests/tests/multiarch/openhcl_servicing.rs | Updates servicing keepalive test to validate fused keepalive device behavior (no post-servicing CREATE_IO_COMPLETION_QUEUE). |
| vm/devices/storage/storage_tests/tests/scsidvd_nvme.rs | Adjusts NvmeDriver::new call site for the new fused-mode argument. |
| vm/devices/storage/disk_nvme/nvme_driver/src/tests.rs | Updates NVMe driver unit tests for the new constructor signature. |
| vm/devices/storage/disk_nvme/nvme_driver/src/queue_pair.rs | Adds an option to disable admin AER issuance via AdminAerHandler. |
| vm/devices/storage/disk_nvme/nvme_driver/src/driver.rs | Core fused keepalive implementation: new driver flag, disables AER + async event task, eagerly initializes IO queues, and extends save/restore with unmapped. |
| vm/devices/storage/disk_nvme/nvme_driver/fuzz/fuzz_nvme_driver.rs | Updates fuzzer harness for the new driver constructor signature. |
| petri/src/vm/hyperv/powershell.rs | Removes the hard error blocking enable_nvme_keepalive for Hyper-V servicing; keepalive is now controlled via the DisableNvmeKeepalive flag. |
| openhcl/underhill_core/src/nvme_manager/mod.rs | Renames/reworks device-ID detection to identify “fused keepalive device” instead of “keepalive incompatible”. |
| openhcl/underhill_core/src/nvme_manager/manager.rs | Propagates fused-device detection through driver lifecycle and adjusts save/shutdown gating logic. |
| openhcl/underhill_core/src/nvme_manager/device.rs | Extends NVMe driver spawner interface to pass fused-mode through to nvme_driver::NvmeDriver create/restore paths. |
Comment on lines
+1520
to
+1523
| self.io_issuers.per_cpu[cpu as usize] | ||
| .set(issuer) | ||
| .ok() | ||
| .unwrap(); |
Comment on lines
+2014
to
+2016
| /// When `true`, interrupt has not been affinitized to a real CPU yet. | ||
| /// Defaults to `false` for backwards compatibility with older versions. | ||
| pub unmapped: bool, |
Comment on lines
+511
to
+512
| /// WORKAROUND: a subset of devices require "fused keepalive". This flag signals to the NVMe | ||
| /// driver that this devices' admin queues may be unusable after a servicing event |
alandau
reviewed
Jun 24, 2026
Comment on lines
+1535
to
+1538
| self.io_issuers.per_cpu[cpu as usize] | ||
| .set(issuer) | ||
| .ok() | ||
| .unwrap(); |
| "enable_nvme_keepalive is not yet supported for HyperV VMs" | ||
| )); | ||
| } | ||
| run_host_cmd( |
Address PR review feedback: log both the fused-keepalive eager pre-creation path and the normal lazy single-queue path. Also document that dropping the returned io queue issuer handle is safe since the owning queue pair is retained in worker.io and Issuer has no Drop side effects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+1521
to
+1544
| match self.device.map_interrupt(iv.into(), cpu) { | ||
| Ok(_interrupt) => { | ||
| 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) | ||
| .ok() | ||
| .unwrap(); | ||
| return; | ||
| } | ||
| Err(err) => { | ||
| tracing::error!( | ||
| cpu, | ||
| iv, | ||
| pci_id = ?self.device.id(), | ||
| error = ?err, | ||
| "fused mode: failed to remap interrupt, falling back" | ||
| ); | ||
| } | ||
| } |
Comment on lines
+613
to
+618
| .with_context(|| { | ||
| format!( | ||
| "failed to create io queue {} (fused keepalive device mode)", | ||
| i | ||
| ) | ||
| })?; |
Comment on lines
+1529
to
+1532
| self.io_issuers.per_cpu[cpu as usize] | ||
| .set(issuer) | ||
| .ok() | ||
| .unwrap(); |
Comment on lines
+511
to
+512
| /// WORKAROUND: a subset of devices require "fused keepalive". This flag signals to the NVMe | ||
| /// driver that this devices' admin queues may be unusable after a servicing event |
Comment on lines
+2022
to
+2025
| #[mesh(4)] | ||
| /// When `true`, interrupt has not been affinitized to a real CPU yet. | ||
| /// Defaults to `false` for backwards compatibility with older versions. | ||
| pub unmapped: bool, |
Comment on lines
+134
to
136
| "failed to read PCI vendor/device IDs; treating device as not a fused keepalive device" | ||
| ); | ||
| false |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
[Do not merge, scale testing is ongoing]
A subset of nvme devices in Azure cannot expect the admin completion queue to be mapped after a servicing event. For those devices, we run in keepalive "fused" mode, where we will not issue any admin commands after nvme driver initialization is complete.
We achieve this by: