Repository navigation
Conversation
ilstam
changed the title
ilstam
marked this pull request as ready for review
September 17, 2026 17:15
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6226 +/- ##
==========================================
+ Coverage 83.05% 83.07% +0.01%
==========================================
Files 277 277
Lines 31481 31511 +30
==========================================
+ Hits 26147 26178 +31
+ Misses 5334 5333 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
ilstam
added this pull request to stack #6230
September 17, 2026 17:16
ilstam
added
the
Status: Awaiting review
Manciukic
reviewed
Oct 2, 2026
| impl PciBuses { | ||
| /// Create the root bus and one secondary bus per root port | ||
| pub fn new(secondary_buses: u8) -> Self { | ||
| let count = usize::from(secondary_buses) + 1; |
Contributor
There was a problem hiding this comment.
nit: Nothing passes more than 0 here yet, but with u8 a caller could ask for up to 255 secondary buses. Anything above 31 has no ECAM coverage, and at 255 num_buses() would hit its expect. Worth catching it at construction time:
Suggested change
| let count = usize::from(secondary_buses) + 1; | |
| assert!( | |
| secondary_buses < MAX_PCI_BUSES, | |
| "PCI segment cannot have more than {} secondary buses", | |
| MAX_PCI_BUSES - 1 | |
| ); | |
| let count = usize::from(secondary_buses) + 1; |
Manciukic
previously approved these changes
Oct 2, 2026
Today Firecracker presents a single PCI bus to the guest, with no Type 1 devices other than the host bridge itself. Virtio devices appear as Root Complex Integrated Endpoints attached directly to the host bridge, on bus 0. Native PCIe hot-plug needs root ports, and each root port starts a secondary bus, so configuration cycles have to be able to reach buses other than 0. Introduce PciBuses holding the buses a segment has; the root bus plus one secondary bus per root port. No visible change for the guest at this point, nothing creates root ports and secondary buses yet. Signed-off-by: Ilias Stamatis <ilstam@amazon.com>
Until now PCI_MMIO_CONFIG_SIZE_PER_SEGMENT was meant to cover every device and function in a single bus because it was assumed that there's a single PCI bus in the system. Since we are going to expand the topology to support up to 32 buses this needs to be updated. Rename PCI_MMIO_CONFIG_SIZE_PER_SEGMENT to PCI_MMIO_CONFIG_SIZE_PER_BUS which is a more accurate name and move the constant to pci/bus.rs instead of repeating its definition per arch. And add a new PCI_MMIO_CONFIG_SIZE_PER_SEGMENT that represents the ECAM space for 32 buses. The ECAM regions grows from 1 MiB to 32 MiB which is 's still well inside the 256 MiB PCI_MMCONFIG_SIZE window which could be used to cover every possible BDF (256 buses). The test_mmio_bus() test used to access the entire 1MiB ECAM region, but that's not possible after growing the ECAM region to 32 MiB since the stack is not large enough to accommodate such a large local variable. Adjust the test to instead check that the beginning and end of the ECAM region can be accessed, whereas accessing just before or just after it fails. Signed-off-by: Ilias Stamatis <ilstam@amazon.com>
Two bus ranges are advertised to the guest via ACPI tables and both describe bus 0 only. Subsequent patches will extend the bus topology so secondary buses need to be advertised too. Make the MCFG ECAM entry cover every bus a PCI segment can possibly have, and add the full bus range of present buses in the host bridge's _CRS. Signed-off-by: Ilias Stamatis <ilstam@amazon.com>
The FDT advertises a single PCI bus to the guest. Change this to advertise the full range of buses present in the system. Signed-off-by: Ilias Stamatis <ilstam@amazon.com>
Add an _OSC method to the host bridge. This can be used by the OS to request it's granted control of certain capabilities (PCIe hotplug, power management events, etc). Otherwise the platform keeps control of them. Grant whatever the guest asks for by returning the capabilities buffer unchanged. Granting control of features Firecracker does not implement is harmless. The guest only drives a feature if it also finds the corresponding PCI capability. Signed-off-by: Ilias Stamatis <ilstam@amazon.com>
ilstam
force-pushed
the
ilstam-hotplug-1
branch
from
October 6, 2026 13:07
93557a0 to
13e76c0
Compare
ShadowCurse
approved these changes
Oct 6, 2026
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
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.
Firecracker currently supports a single PCI bus. This PR is the groundwork for extending that to 32 buses.