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

Remove multi irq support for mmio devices #4534

Open
3 tasks
ShadowCurse opened this issue Apr 2, 2024 · 4 comments · May be fixed by #4601
Open
3 tasks

Remove multi irq support for mmio devices #4534

ShadowCurse opened this issue Apr 2, 2024 · 4 comments · May be fixed by #4601
Labels
Good first issue Indicates a good issue for first-time contributors Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` rust Pull requests that update Rust code Status: Parked Indicates that an issues or pull request will be revisited later Type: Enhancement Indicates new feature requests

Comments

@ShadowCurse
Copy link
Contributor

ShadowCurse commented Apr 2, 2024

Currently Firecracker have an ability to create multiple irqs for MMIO devices. These irqs are store in the MMIODeviceInfo struct here.
As per discussion here we would like to remove this "multi irq" configuration as Firecracker never uses for more than 1 irq per device. Also MMIO devices can not have more than 1 irq anyway.

The change can be as simple as replacing Vec<u32> with Option<NonZeroU32> and updating all places where this field is used.

Checks

  • Have you searched the Firecracker Issues database for similar requests?
  • Have you read all the existing relevant Firecracker documentation?
  • Have you read and understood Firecracker's core tenets?
@ShadowCurse ShadowCurse added Good first issue Indicates a good issue for first-time contributors Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` Type: Enhancement Indicates new feature requests rust Pull requests that update Rust code labels Apr 2, 2024
@andr3wy
Copy link
Contributor

andr3wy commented Apr 3, 2024

Hi everyone,

We're a group at UT Austin and we're interested in picking this issue up.

@ShadowCurse
Copy link
Contributor Author

Hi @andr3wy, you are welcome to work on this.

@JonathanWoollett-Light JonathanWoollett-Light linked a pull request May 13, 2024 that will close this issue
9 tasks
@roypat roypat linked a pull request May 16, 2024 that will close this issue
9 tasks
@zulinx86 zulinx86 added the Status: Parked Indicates that an issues or pull request will be revisited later label Jun 20, 2024
@ShadowCurse
Copy link
Contributor Author

If anyone is interested in taking this issue, you can reuse already existing PR @andr3wy created: #4601

@roypat
Copy link
Contributor

roypat commented Oct 16, 2024

If anyone is interested in taking this issue, you can reuse already existing PR @andr3wy created: #4601

Note that the implementation there is completely done, but for some reason the CI fails which we havent had a chance to look into yet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good first issue Indicates a good issue for first-time contributors Priority: Low Indicates that an issue or pull request should be resolved behind issues or pull requests labelled ` rust Pull requests that update Rust code Status: Parked Indicates that an issues or pull request will be revisited later Type: Enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants