Skip to content

Conversation

@seijikun
Copy link
Contributor

The bridge detection to decide whether we want to recurse to a subsequent bus was broken due to two reasons:

  • Register struct PciHeader1Register6 had the wrong order, which caused the range (start - end) of subsequent bus numbers to be swapped
  • Forgot to mask the header_type with 0b01111111, because the most significant bit specifies whether the device is a multi-function device.

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

Copy link
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix! Do you see the potential for a integration test?

@seijikun
Copy link
Contributor Author

Thanks for the fix! Do you see the potential for a integration test?

Yes, but not yet.
First, I would like to add DevicePath generation for every found PCI device.
(That's what this MR works towards)
With those, writing a pci integration tests should be much more robust.

@phip1611
Copy link
Member

phip1611 commented Nov 21, 2025

What is currently the blocker when it comes to device path generation for you? @nicholasbishop created significant groundwork for constructing device paths?

@seijikun
Copy link
Contributor Author

What is currently the blocker when it comes to device path generation for you? @nicholasbishop created significant groundwork for constructing device paths?

Mostly that it already was 03:00 AM "yesterday".

@seijikun
Copy link
Contributor Author

Can we do the DevicePath generation and subsequentely the integration test in a separate MR?
I will probably need some support with the DevicePath API.

@seijikun
Copy link
Contributor Author

seijikun commented Nov 21, 2025

Ah, just for the record:
The integration test would not have caught this bug. At no point did the current implementation yield incorrect results.

The broken PCI bridge recursion was compensated for by iterating through all bus numbers of a PciRoot in PciRootBridgeIo::enumerate(). I had mistakenly assumed that this was necessary - until I tried to generate device paths and discovered the broken recursion.

@phip1611 phip1611 added this pull request to the merge queue Nov 21, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 21, 2025
@phip1611 phip1611 added this pull request to the merge queue Nov 21, 2025
Merged via the queue into rust-osdev:main with commit 43222e3 Nov 21, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants