uefi: Refactor PciRootBridgeIo::enumerate() with tree-topology information #1830
Conversation
seijikun
force-pushed
the
mr-pci-enumerate3
branch
5 times, most recently
from
2a46de0 to
fbed494
Compare
phip1611
left a comment
phip1611
left a comment
There was a problem hiding this comment.
Thanks for working on this! My main concern is my lack of domain knowledge of PCIe. Please elaborate a little more to help me understand what's going on :)
| // test is more interesting. | ||
| cmd.args([ | ||
| "-device", | ||
| "ioh3420,id=root_port1,bus=pcie.0", |
There was a problem hiding this comment.
I'm not that familiar with PCI. Could you please elaborate a little?
- why adding a root port? Is this equivalent to adding a new PCI bus?
- what is
x3130-upstreamand why are you using it here? - what is
x3130-downstreamand why are you using it here? virtio-scsi-pciis just a random PCI device without serving a functionality here except for appearing on the PCIe bus?- why are you specifying
chassis=9|10?
There was a problem hiding this comment.
Much of this is qemu specific syntax:
ioh3420 reserves a port in pcie.0 - the pcie root port, also called pcie root complex. I imagine this like soldering a PCIe socket on your mainboard, where you can plug stuff in.
Into this socket, we then plug a x3130-upstream. That's a PCIe switch.
x3130-downstream is the other side of the swich, where you can plug child devices in.
I have no idea what chassis is, the way I understood it, it can be any number as long as they are unique. All bridges/switches have this obligatory parameter.
The scsi devices are just for testing, like you said.
Difference PCIe switches and PCIe bridges:
Both allocate a new subordinate bus. Both allow connecting devices to them, which they "pass through" to the port where they are connected upstream. The key difference is, that for you to pass 2 lanes through a bridge, it needs to be connected with 2 lanes to its upstream. Whereas PCIe switches can do switching. They can tunnel y downstream lanes through x upstream lanes - sacrificing speed if y > x (like with network switches). They are rather seldom, which is why I wanted to test it.
There was a problem hiding this comment.
https://blogs.oracle.com/linux/a-study-of-the-linux-kernel-pci-subsystem-with-qemu
| "PCI Device: [{bus}, {dev}, {fun}]: vendor={vendor_id:04X}, device={device_id:04X}, class={class_code:02X}, subclass={subclass_code:02X}" | ||
| "PCI Device: [{bus:02x}, {dev:02x}, {fun:02x}]: vendor={vendor_id:04X}, device={device_id:04X}, class={class_code:02X}, subclass={subclass_code:02X}" | ||
| ); | ||
| for child_bus in pci_tree.iter_subsequent_busses_for(addr) { |
There was a problem hiding this comment.
instead of just iterating, would it make sense to explictely check for the expected PCIe devices?
There was a problem hiding this comment.
The bus numbers are dynamically allocated and thus might vary.
But I've done something like you suggest in the follow-up MR here, based on device paths: 582714a
seijikun
force-pushed
the
mr-pci-enumerate3
branch
from
fbed494 to
602c776
Compare
…ation - Refactored return type from standard BTreeSet to custom PciTree struct - Removed special FullPciIoAddress type, since segment number is PciRoot dependent - During enumeration, skip branches we have already seen - During enumeration, collect tree topology information (which child bus linked from where) - Add complicated pci structure in integration test vm - Print child busses for every device entry in integration test
seijikun
force-pushed
the
mr-pci-enumerate3
branch
from
602c776 to
a82b06c
Compare
2 participants
BTreeSet<PciIoAddress>to customPciTreestruct with tree topology informationPreviously, we iterated over the bus range from the ACPI descriptor. I changed this in my last MR to only visit the first in the range. I changed this back to iterate over all of the entries, but skip the ones we have seen previously - now that that's easily possible. I decided that better safe than sorry is the best way. The iteration over all child busses already saved our ass once - let's not willingly take this safety net away. Sorry for the back and forth here.
The plan for device paths is now to add something like:
PciTree::device_path(&self, addr: PciIoAddress) -> PoolDevicePath.Checklist