-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
#4534: Remove multi irq support for mmio devices #4601
base: main
Are you sure you want to change the base?
Conversation
fec52fa
to
a1d2133
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4601 +/- ##
==========================================
- Coverage 84.32% 84.31% -0.01%
==========================================
Files 249 249
Lines 27512 27516 +4
==========================================
+ Hits 23200 23201 +1
- Misses 4312 4315 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @andr3wy,
Thanks for your PR! There's a couple formatting and build issues on ARM that'll need to be fixed (see https://buildkite.com/firecracker/firecracker-pr/builds/9851 for what exactly is failing). For the formatting, you can run ./tools/devtool fmt
to fix it up (and to check that the style checks will all pass, you can run ./tools/devtool checkstyle
). For the build error on ARM, you can do cargo check --target aarch64-unknown-linux-gnu
. That should allow you to validate the build without needing a cross-compilation toolchain. Also, please squash all the commits into a single one, and give it a meaningful description. Thanks!
2bf0526
to
c9fe5ad
Compare
Issues should be resolved. |
79d925a
to
4d1e052
Compare
Thanks for squashing the commits! It seems that a rebase went a bit awry though, your branch now has some commits that are already on Also I think there's still a compilation issue on ARM, could you have a look at that, too? |
Wanted to write that I won't have access to a computer for at least a month (out of town), so this will have to be on the back burner for a while. |
Related to #4534 |
Hi @andr3wy, |
5779211
to
810402e
Compare
Hi @andr3wy, |
69b8c2c
to
d7c0a7d
Compare
838e581
to
64264d4
Compare
Each MMIO device in Firecracker only utilizes at most one irq line, so capture this property at the type level. Signed-off-by: Andrew Yao <[email protected]> Signed-off-by: Patrick Roy <[email protected]>
Changes
-Utilized Option instead of Vector to store IRQ lines for MMIO devices.
-Add test to test this.
-Update existing tests
...
Reason
Currently, Firecracker have an ability to create multiple irqs for MMIO devices; however, no more than one IRQ is ever used. Therefore, it was suggested to change this to use an Option instead of a vector of length 0 or 1.
...
License Acceptance
By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache 2.0 license. For more information on following Developer
Certificate of Origin and signing off your commits, please check
CONTRIBUTING.md
.PR Checklist
PR.
CHANGELOG.md
.TODO
s link to an issue.contribution quality standards.
rust-vmm
.