Skip to content

feat(pci,virtio): read x86 legacy IRQ via PCI config space#18

Closed
sunhaosheng wants to merge 2 commits intoarceos-org:mainfrom
sunhaosheng:tcp-fix
Closed

feat(pci,virtio): read x86 legacy IRQ via PCI config space#18
sunhaosheng wants to merge 2 commits intoarceos-org:mainfrom
sunhaosheng:tcp-fix

Conversation

@sunhaosheng
Copy link
Copy Markdown

Related Issue

Starry-OS/StarryOS#129

On x86_64, a host-side TCP client could not complete a TCP echo exchange with a server running inside the system. The server never reported an accepted connection, the client blocked indefinitely, and the server could not be terminated with Ctrl+C. See the issue for full reproduction steps.

Description

This PR fixes x86 legacy IRQ discovery for VirtIO PCI devices.

Previously, the x86 path relied on derived or hardcoded IRQ assumptions, which do not reliably match the actual platform-assigned legacy INTx routing. On x86, the correct legacy IRQ should be taken from the PCI configuration space Interrupt Line register instead.

To support this, this PR adds a small PCI config-space accessor in axdriver_pci and uses it in axdriver_virtio to fetch the IRQ from PCI config space during device probing.

Implementation

This PR includes two changes:

  • Add PciConfigAccess to axdriver_pci

    • This provides read/write access to arbitrary PCI configuration space registers through the existing ECAM/MMIO mapping.
    • It is intended as a minimal helper for drivers that need config-space fields not exposed by virtio-drivers.
  • Update axdriver_virtio::probe_pci_device() on x86

    • Accept a PciConfigAccess reference.
    • Read the legacy IRQ from PCI config offset 0x3C (Interrupt Line).
    • Reject invalid/unassigned values (0 and 0xff) on x86 instead of treating them as usable IRQs.

Additional Context

This change is based on the same fix direction used in x-kernel commit, where x86 VirtIO probing was changed to use the PCI Interrupt Line register instead of hardcoded IRQ derivation.

I kept this PR intentionally small so it can be reviewed independently from any follow-up work around MSI-X or higher-level interrupt handling.

Add PciConfigAccess to axdriver_pci so drivers can read arbitrary
PCI config registers, and use it in axdriver_virtio to fetch the
legacy IRQ from the Interrupt Line register on x86.

This avoids hardcoded IRQ derivation and matches the platform-assigned
legacy INTx routing.
@AsakuraMizu
Copy link
Copy Markdown
Contributor

AsakuraMizu commented Mar 10, 2026

Merge to https://github.com/arceos-org/axdriver_crates/tree/dev first? Note that you can change the target branch without re-creating the PR.

@sunhaosheng
Copy link
Copy Markdown
Author

Merge to https://github.com/arceos-org/axdriver_crates/tree/dev first? Note that you can change the target branch without re-creating the PR.

It seems I don't have permission to change the target branch. Could you help me make that change?

@AsakuraMizu
Copy link
Copy Markdown
Contributor

It seems I don't have permission to change the target branch. Could you help me make that change?

I can't operate it either. I don't know why. Something might be wrong.

unsafe impl Send for PciConfigAccess {}
unsafe impl Sync for PciConfigAccess {}

impl PciConfigAccess {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why not reuse Transport::read_config_space from the virtio-drivers crate?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This change needs the PCI config-space Interrupt Line register (0x3C), not the VirtIO device-specific config space exposed by the transport. Transport::config_space is for VirtIO device config, while PciConfigAccess is for PCI header/config-space access during PCI probe.

@sunhaosheng
Copy link
Copy Markdown
Author

sunhaosheng commented Mar 10, 2026

This PR is moved to
#19

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.

4 participants