pci_core: add infrastructure for devices to advertise pcie support with some basic functionality #1930
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds infrastructure for PCI devices to advertise PCIe support by implementing the PCI Express capability structure. The implementation includes basic Device Capabilities, Device Control, and Device Status registers, along with support for Function Level Reset (FLR) functionality.
Key changes:
- Adds PCI Express capability ID and register structures to the PCI specification
- Implements PciExpressCapability with FLR support and proper register handling
- Provides comprehensive test coverage for the new capability functionality
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| vm/devices/pci/pci_core/src/spec.rs | Adds PCI_EXPRESS capability ID and PCIe register bitfield structures |
| vm/devices/pci/pci_core/src/capabilities/pci_express.rs | Implements complete PciExpressCapability with FLR handler support |
| vm/devices/pci/pci_core/src/capabilities/mod.rs | Exports the new pci_express capability module |
Comments suppressed due to low confidence (1)
vm/devices/pci/pci_core/src/capabilities/pci_express.rs:1
- There is trailing whitespace after 'Register' in the comment.
// Copyright (c) Microsoft Corporation.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Matt LaFayette (Kurjanowicz) <[email protected]>
Co-authored-by: Matt LaFayette (Kurjanowicz) <[email protected]>
| type SavedState = state::SavedState; | ||
|
|
||
| fn save(&mut self) -> Result<Self::SavedState, SaveError> { | ||
| let state = self.state.lock(); |
There was a problem hiding this comment.
I suggest destructuring both self and state in save to make sure new fields don't get overlooked. The same fr SavedState in restore.
There was a problem hiding this comment.
This was a really good point. I realized that the save/restore was not taking in to account the flr handler callback which we can't save rn. Removing the save/restore functionality for now because this will only be used in the fault controller that also doesn't support save/restore for the time being
| /// | 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 | |
There was a problem hiding this comment.
thanks for fixing this up :)
Adds the PCI_Express capability option for devices to advertise PCIe support. This change will also us to add FLR support to the nvme_test crate for further perf testing.
PciExpressCapability: New capability struct implementing PCI Express Device Capabilities, Device Control, and Device Status registers
Spec digarams (From PCIe Base Spec 6.4) for reference:
Being made as a helper PR to #1858 since that was getting a little too large/complicated