#4534: Remove multi irq support for mmio devices #4601
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
andr3wy
force-pushed
the
4534
branch
2 times, most recently
from
fec52fa
to
a1d2133
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4601 +/- ##
==========================================
+ Coverage 82.14% 82.41% +0.26%
==========================================
Files 255 228 -27
Lines 31285 29048 -2237
==========================================
- Hits 25700 23940 -1760
+ Misses 5585 5108 -477
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
roypat
reviewed
May 7, 2024
There was a problem hiding this comment.
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!
This was referenced May 7, 2024
andr3wy
force-pushed
the
4534
branch
3 times, most recently
from
2bf0526
to
c9fe5ad
Compare
|
Issues should be resolved. |
Signed-off-by: Andrew Yao <andr3wy@gmail.com>
Import kvm_bindings items instead of referring them by full name. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Previously we were storing kvi for vcpus only when cpu template was used. This was done to save vcpu configuration for snapshot restoration step, because when vcpus are restored, we don't have access to the cpu template to apply changes again. But when cpu template was not used we did not store already created kvi and were getting new one from KVM. This is a redundant work which we can avoid if we store kvi all the time. Also storing kvi unconditionally simplifies the code significantly. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
This reverts commit 6624eff. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
This reverts commit 238f55c. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
If test uses snapshots and fails save all files from `root` of a vm into the `test_results`. Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
KVM passes through bit 27 of the `MSR_IA32_ARCH_CAPABILITIES` MSR (0x10A) to the guest, to let them know whether the processor they're running on is affected by RFDS. According to Intel, only Atom processors are affected [[1]]. Our baselines for 6.1 kernel are already updated. Now this change was ported back to 5.10, so we update baselines for it. [1]: https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/advisory-guidance/register-file-data-sampling.html Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Include which error one would see if one forgets to set the `AWS_EMF_NAMESPACE` and `AWS_EMF_ENVIRONMENT` variables to `local`. Note a common pitfall of trying to set environment variables through buildkite. Signed-off-by: Patrick Roy <roypat@amazon.co.uk>
This is a follow up to firecracker-microvm#4593 Signed-off-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Modified the following error types to derive thiserror::Error and displaydoc::Display: - MainError - RunWithoutApiError - FileError - FilterError - EditMemoryError - VcpuError - BlockIoError - DumpCpuConfigError - BuildMicrovmFromRequestsError - BalloonConfigError - DriveError - VsockConfigError - MemoryError - JailerError This change allows for more detailed error messages to be displayed when these errors are encountered. Also updated test cases to match the new error type output. Signed-off-by: Eddie Cazares <Ecazares15@utexas.edu> Co-authored-by: Lindsey Bowen <lindseyb803@gmail.com> Co-authored-by: Thomas Bunch <tebunch@utexas.edu>
Bumps the firecracker group with 19 updates: | Package | From | To | | --- | --- | --- | | [zerocopy](https://github.com/google/zerocopy) | `0.7.32` | `0.7.33` | | [serde](https://github.com/serde-rs/serde) | `1.0.199` | `1.0.200` | | [serde_derive](https://github.com/serde-rs/serde) | `1.0.199` | `1.0.200` | | [aws-lc-rs](https://github.com/awslabs/aws-lc-rs) | `1.7.0` | `1.7.1` | | [base64](https://github.com/marshallpierce/rust-base64) | `0.22.0` | `0.22.1` | | [anstream](https://github.com/rust-cli/anstyle) | `0.6.13` | `0.6.14` | | [anstyle](https://github.com/rust-cli/anstyle) | `1.0.6` | `1.0.7` | | [anstyle-parse](https://github.com/rust-cli/anstyle) | `0.2.3` | `0.2.4` | | [anstyle-query](https://github.com/rust-cli/anstyle) | `1.0.2` | `1.0.3` | | [anstyle-wincon](https://github.com/rust-cli/anstyle) | `3.0.2` | `3.0.3` | | [autocfg](https://github.com/cuviper/autocfg) | `1.2.0` | `1.3.0` | | [aws-lc-fips-sys](https://github.com/aws/aws-lc-rs) | `0.12.7` | `0.12.8` | | [aws-lc-sys](https://github.com/aws/aws-lc-rs) | `0.15.0` | `0.16.0` | | [cc](https://github.com/rust-lang/cc-rs) | `1.0.96` | `1.0.97` | | [colorchoice](https://github.com/rust-cli/anstyle) | `1.0.0` | `1.0.1` | | [getrandom](https://github.com/rust-random/getrandom) | `0.2.14` | `0.2.15` | | [num-traits](https://github.com/rust-num/num-traits) | `0.2.18` | `0.2.19` | | [winnow](https://github.com/winnow-rs/winnow) | `0.6.7` | `0.6.8` | | [zerocopy-derive](https://github.com/google/zerocopy) | `0.7.32` | `0.7.33` | Updates `zerocopy` from 0.7.32 to 0.7.33 - [Release notes](https://github.com/google/zerocopy/releases) - [Changelog](https://github.com/google/zerocopy/blob/main/CHANGELOG.md) - [Commits](google/zerocopy@v0.7.32...v0.7.33) Updates `serde` from 1.0.199 to 1.0.200 - [Release notes](https://github.com/serde-rs/serde/releases) - [Commits](serde-rs/serde@v1.0.199...v1.0.200) Updates `serde_derive` from 1.0.199 to 1.0.200 - [Release notes](https://github.com/serde-rs/serde/releases) - [Commits](serde-rs/serde@v1.0.199...v1.0.200) Updates `aws-lc-rs` from 1.7.0 to 1.7.1 - [Release notes](https://github.com/awslabs/aws-lc-rs/releases) - [Commits](aws/aws-lc-rs@v1.7.0...v1.7.1) Updates `base64` from 0.22.0 to 0.22.1 - [Changelog](https://github.com/marshallpierce/rust-base64/blob/master/RELEASE-NOTES.md) - [Commits](marshallpierce/rust-base64@v0.22.0...v0.22.1) Updates `anstream` from 0.6.13 to 0.6.14 - [Commits](rust-cli/anstyle@anstream-v0.6.13...anstream-v0.6.14) Updates `anstyle` from 1.0.6 to 1.0.7 - [Commits](rust-cli/anstyle@v1.0.6...v1.0.7) Updates `anstyle-parse` from 0.2.3 to 0.2.4 - [Commits](rust-cli/anstyle@anstyle-parse-v0.2.3...anstyle-parse-v0.2.4) Updates `anstyle-query` from 1.0.2 to 1.0.3 - [Commits](rust-cli/anstyle@anstyle-query-v1.0.2...anstyle-query-v1.0.3) Updates `anstyle-wincon` from 3.0.2 to 3.0.3 - [Commits](rust-cli/anstyle@anstyle-wincon-v3.0.2...anstyle-wincon-v3.0.3) Updates `autocfg` from 1.2.0 to 1.3.0 - [Commits](cuviper/autocfg@1.2.0...1.3.0) Updates `aws-lc-fips-sys` from 0.12.7 to 0.12.8 - [Release notes](https://github.com/aws/aws-lc-rs/releases) - [Commits](aws/aws-lc-rs@aws-lc-fips-sys/v0.12.7...aws-lc-fips-sys/v0.12.8) Updates `aws-lc-sys` from 0.15.0 to 0.16.0 - [Release notes](https://github.com/aws/aws-lc-rs/releases) - [Commits](aws/aws-lc-rs@aws-lc-sys/v0.15.0...aws-lc-sys/v0.16.0) Updates `cc` from 1.0.96 to 1.0.97 - [Release notes](https://github.com/rust-lang/cc-rs/releases) - [Commits](rust-lang/cc-rs@1.0.96...1.0.97) Updates `colorchoice` from 1.0.0 to 1.0.1 - [Commits](rust-cli/anstyle@colorchoice-v1.0.0...colorchoice-v1.0.1) Updates `getrandom` from 0.2.14 to 0.2.15 - [Changelog](https://github.com/rust-random/getrandom/blob/master/CHANGELOG.md) - [Commits](rust-random/getrandom@v0.2.14...v0.2.15) Updates `num-traits` from 0.2.18 to 0.2.19 - [Changelog](https://github.com/rust-num/num-traits/blob/master/RELEASES.md) - [Commits](rust-num/num-traits@num-traits-0.2.18...num-traits-0.2.19) Updates `winnow` from 0.6.7 to 0.6.8 - [Changelog](https://github.com/winnow-rs/winnow/blob/main/CHANGELOG.md) - [Commits](winnow-rs/winnow@v0.6.7...v0.6.8) Updates `zerocopy-derive` from 0.7.32 to 0.7.33 - [Release notes](https://github.com/google/zerocopy/releases) - [Changelog](https://github.com/google/zerocopy/blob/main/CHANGELOG.md) - [Commits](google/zerocopy@v0.7.32...v0.7.33) --- updated-dependencies: - dependency-name: zerocopy dependency-type: direct:production update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: serde dependency-type: direct:production update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: serde_derive dependency-type: direct:production update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: aws-lc-rs dependency-type: direct:production update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: base64 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: anstream dependency-type: indirect update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: anstyle dependency-type: indirect update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: anstyle-parse dependency-type: indirect update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: anstyle-query dependency-type: indirect update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: anstyle-wincon dependency-type: indirect update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: autocfg dependency-type: indirect update-type: version-update:semver-minor dependency-group: firecracker - dependency-name: aws-lc-fips-sys dependency-type: indirect update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: aws-lc-sys dependency-type: indirect update-type: version-update:semver-minor dependency-group: firecracker - dependency-name: cc dependency-type: indirect update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: colorchoice dependency-type: indirect update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: getrandom dependency-type: indirect update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: num-traits dependency-type: indirect update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: winnow dependency-type: indirect update-type: version-update:semver-patch dependency-group: firecracker - dependency-name: zerocopy-derive dependency-type: indirect update-type: version-update:semver-patch dependency-group: firecracker ... Signed-off-by: dependabot[bot] <support@github.com>
If a test fails, the FCmetricsMonitor keeps running in some tests. After we kill the microvm and remove the resources, the monitor complains it cannot find the metrics file. This is tricky to fix simply since we don't have a handle to the monitor either at fixture teardown or in the Microvm class. Add a hook in the Microvm class to register monitors, so we can stop all of them before killing the microvm. Also adapt existing monitors to fix this pattern. Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
The current test failure metrics have high cardinality because they include the test name. This makes it hard to visualize them in CloudWatch since we run into aggregation limits. It would be nice to have one global failure rate, and some views per-kernel and per-CPU type. This adds 10 new metrics = 6 per-CPU + 3 per-host kernel + 1 global metric so it is not too costly. Signed-off-by: Pablo Barbáchano <pablob@amazon.com>
|
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 |
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.TODOs link to an issue.contribution quality standards.
rust-vmm.