Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lspci app, restructure PCI legacy interrupts (INTx) initialization #1081

Merged
merged 54 commits into from
Dec 22, 2023

Conversation

NathanRoyer
Copy link
Member

(This PR depends on #1071)

This adds an lspci command:

image

It takes no argument and always lists all devices.

Copy link
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

The lspci app itself is fine. I left some comments about other PCI changes, the most significant of which is how the INTx interrupt handlers are initialized (and how it differs across architectures).

kernel/pci/src/lib.rs Outdated Show resolved Hide resolved
kernel/pci/src/lib.rs Outdated Show resolved Hide resolved
kernel/pci/src/lib.rs Outdated Show resolved Hide resolved
kernel/pci/src/lib.rs Outdated Show resolved Hide resolved
@kevinaboos kevinaboos changed the title lspci command Add lspci app, restructure PCI legacy interrupts (INTx) initialization Dec 22, 2023
Copy link
Member

@kevinaboos kevinaboos left a comment

Choose a reason for hiding this comment

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

Thanks, I've accepted it and left a comment for a minor future improvement. Let me know your thoughts on that.

Comment on lines +245 to +246
let intx_numbers = INTX_NUMBERS.lock();
intx_numbers[$num].expect("uninitialized x86 PCI INTx handler")
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit funky and kinda hard to read; I think this is a good place to just include this logic in the actual body of the interrupt handler and issue an EOI directly in the body instead of returning HandlerDidNotSendEoi.

Another thing we could do is to include the interrupt number alongside the waker instance or alongside the INTX_DEVICES static, in order to "ensure" those are always available.

@kevinaboos kevinaboos merged commit ac5712c into theseus-os:theseus_main Dec 22, 2023
3 checks passed
github-actions bot pushed a commit that referenced this pull request Dec 22, 2023
)

* INTX interrupt handler registration for PCI devices is now done
  lazily on both architectures.
  * On aarch64, the interrupt numbers are known statically.
  * On x86_64, we expect that the driver or module using the
    PCI device knows which interrupt number to use.
    Technically this information can be discovered through ACPI AML,
    but we do not yet support that because it is tedious and difficult.

* Clarify PCI interrupt information functions, e.g., MSI/MSI-x support, etc.

Co-authored-by: Kevin Boos <1139460+kevinaboos@users.noreply.github.com> ac5712c
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