Minor cleanup in PCI code #5342
Merged
Conversation
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
We are vending PCI code taken from Cloud Hypervisor's implementation. There is code in there that we don't actually use. So drop it to reduce the dead code in the project. Signed-off-by: Babis Chalios <[email protected]>
Cloud Hypervisor code is passing a resource type argument in the logic that allocates BARs for devices. `Resource` is an Enum where one of its variants is `PciBar`. Not sure what Cloud Hypervisor uses this for, but it seems redundant since this method is specifically used to allocate BAR memory for devices. We definitely don't use it, so remove it. Signed-off-by: Babis Chalios <[email protected]>
Despite the fact we are using a single 64bit MMIO BARs for VirtIO devices, we had code that allowed for multiple BARs including BARs of other types (IO and 32bit MMIO). Remove this code and always assume we are only using 64bit BAR at index 0. Signed-off-by: Babis Chalios <[email protected]>
bchalios
added
the
Status: Awaiting review
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/pcie #5342 +/- ##
================================================
+ Coverage 80.36% 80.68% +0.32%
================================================
Files 265 265
Lines 30828 30650 -178
================================================
- Hits 24775 24731 -44
+ Misses 6053 5919 -134
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
roypat
approved these changes
Aug 1, 2025
Manciukic
approved these changes
Aug 1, 2025
bchalios
merged commit 97eaa9e
into
firecracker-microvm:feature/pcie
7 of 8 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Changes
Remove unused code from
pcicrate. Also, simplify code in VirtIO PCI transport introducing stronger assumptions about the fact that we are always using a single 64bit BAR for VirtIO devicesReason
Simplify code and remove dead code.
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md.PR Checklist
tools/devtool checkbuild --allto verify that the PR passesbuild checks on all supported architectures.
tools/devtool checkstyleto verify that the PR passes theautomated style checks.
how they are solving the problem in a clear and encompassing way.
in the PR.
CHANGELOG.md.Runbook for Firecracker API changes.
integration tests.
TODO.rust-vmm.