nvme: Add Function Level Reset (FLR) support to NVMe emulator #1720
Conversation
- Add PCI Express capability ID to pci_core spec - Implement PciExpressCapability with FLR support in pci_core - Add FlrHandler trait for handling FLR events - Add flr_support flag to NvmeControllerCaps and NvmeControllerHandle - Wire FLR capability to NvmeController when enabled - Implement FLR reset logic using atomic flag mechanism - Add comprehensive unit tests for FLR functionality - Update nvme_resources to include FLR configuration option Co-authored-by: mattkur <13772048+mattkur@users.noreply.github.com>
Copilot
AI
changed the title
| pub fn new(flr_supported: bool, flr_handler: Option<Arc<dyn FlrHandler>>) -> Self { | ||
| let device_capabilities = pci_express::DeviceCapabilities::new() | ||
| .with_function_level_reset(flr_supported) | ||
| .with_max_payload_size(0) // 128 bytes |
There was a problem hiding this comment.
How did you get 128 bytes here?
There was a problem hiding this comment.
The value 0 for max_payload_size follows the PCIe specification where 0 encodes 128 bytes. Updated the comment to clarify this mapping.
| device_control | (device_status << 16) | ||
| } | ||
| _ => { | ||
| tracelimit::warn_ratelimited!(offset, "unhandled pci express capability read"); |
There was a problem hiding this comment.
Please include the label in all tracing from this cap. E.g. warn_ratelimited!(?label, offset...
There was a problem hiding this comment.
Added the capability label to all tracing calls in both read_u32 and write_u32 methods for better debugging context.
| /// * `flr_handler` - Optional handler to be called when FLR is initiated | ||
| pub fn new(flr_supported: bool, flr_handler: Option<Arc<dyn FlrHandler>>) -> Self { | ||
| let device_capabilities = pci_express::DeviceCapabilities::new() | ||
| .with_function_level_reset(flr_supported) |
There was a problem hiding this comment.
This is verbose, try to use FromZeroes or default (and then just override the one bit you need)
There was a problem hiding this comment.
Simplified the DeviceCapabilities initialization to use the default state from new() and only set the function_level_reset bit as needed, eliminating the verbose builder pattern.
| } | ||
| } | ||
|
|
||
| fn write_u32(&mut self, offset: u16, val: u32) { |
There was a problem hiding this comment.
Please add unit test for read_u32 and write_u32 in this file (add the tests in this file, too)
There was a problem hiding this comment.
Added comprehensive unit tests for read_u32 and write_u32 methods covering all register offsets, FLR functionality, error handling, and edge cases including readonly register protection and unhandled offsets.
vm/devices/pci/pci_core/src/spec.rs
Outdated
| /// Offsets into the PCI Express Capability Header | ||
| /// | ||
| /// Table pulled from PCI Express Base Specification Rev. 3.0 | ||
| /// | ||
| /// | Offset | Bits 31-24 | Bits 23-16 | Bits 15-8 | Bits 7-0 | | ||
| /// |-----------|------------------|------------------|------------------|------------------| | ||
| /// | Cap + 0x0 | PCI Express Capabilities Register | Next Pointer | Capability ID (0x10) | | ||
| /// | Cap + 0x4 | Device Capabilities Register | | ||
| /// | Cap + 0x8 | Device Status | Device Control | | ||
| /// | Cap + 0xC | Link Capabilities Register | | ||
| /// | Cap + 0x10| Link Status | Link Control | |
There was a problem hiding this comment.
Cleaned up the table formatting by removing extra space in the PCI Express Capabilities Register cell to align properly with the column headers.
Co-authored-by: mattkur <13772048+mattkur@users.noreply.github.com>
This PR adds Function Level Reset (FLR) support to the NVMe emulator to enable VFIO to set the
reset_methodfile in sysfs.Problem
When testing with the NVMe emulator, VFIO didn't allow OpenHCL to modify the
reset_methodfile because the emulator didn't advertise FLR support. This prevented proper device reset functionality in virtualized environments.Solution
Added a minimal PCI Express capability implementation that:
Key Components
flr_supportflag toNvmeControllerCapsandNvmeControllerHandleTesting
Added comprehensive unit tests that verify:
Usage
Fixes #1719.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.