Enhance usability of CPU template helper tool #4492
Merged
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
zulinx86
requested review from
xmarcalx,
kalyazin,
pb8o and
wearyzen
as code owners
zulinx86
force-pushed
the
improve_helper
branch
3 times, most recently
from
fbb59af to
b7f39dd
Compare
zulinx86
added
rust
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4492 +/- ##
==========================================
+ Coverage 81.52% 81.56% +0.04%
==========================================
Files 243 243
Lines 29848 29875 +27
==========================================
+ Hits 24334 24368 +34
+ Misses 5514 5507 -7
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
zulinx86
force-pushed
the
improve_helper
branch
2 times, most recently
from
baa2784 to
cda58bd
Compare
pb8o
reviewed
Mar 11, 2024
Yes, we can keep the source. I'm still trying to find a way to create a PE binary instead of copying it from linux-loader binary in order to make it under our control. |
zulinx86
force-pushed
the
improve_helper
branch
8 times, most recently
from
be59e62 to
93de09b
Compare
zulinx86
added
the
Status: Awaiting review
pb8o
previously approved these changes
Mar 15, 2024
ShadowCurse
reviewed
Mar 15, 2024
The `--config` parameter of the fingerprint dump command is used to pass a Firecracker config file (not a fingerprint file). Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Imports the template module directly from vmm-sys-util instead of firecracker's utils library, because it is just re-published from vmm-sys-utils by firecracker's utils library. Thanks to this, we can avoid module name collision between firecracker and cpu-template-helper. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
These type annotations are not needed thanks for Rust's type inference. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
zulinx86
force-pushed
the
improve_helper
branch
3 times, most recently
from
dde4d8f to
d56f643
Compare
The KVM register ID for PC (program counter) was calculated at runtime. Make it constant value like other registers. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Align the register name to Linux kernel. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
On kernel 6.5, a new timer register SYS_CNTPCT_EL0 becomes available and it was added to an exception list in integration tests. But from users' perspective, it is quite harmful for users to need to filter it out on their side because it isn't useful in CPU template lifecycle and it is ignoreable. So it should be excluded on the tool side. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Currently cpu-template-helper requires some resources (kernel, rootfs and Firecracker config files) to dump a guest CPU config or a fingerprint or verify a CPU template, although they don't affect the output essentially. This is because it utilizes `build_microvm_for_boot()` function imported from vmm crate that requires those resources. Adding mock kernels is preliminary task to remove the requirement. `build_microvm_for_boot()` requires a valid ELF file for x86_64 and a file containing a valid kernel header for aarch64. So we use a minimal C program for x86_64 and just write a kernel header to a file for aarch64. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
The added utility function prepares all the required resources temporarily and removes them after use to help users not to need to do that. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Now users no longer need to prepare kernel image file, rootfs file and Firecracker config file that don't affect the output essentially. If `--config` is not provided, the CPU template helper tool does the preparation instead. The parameter itself is preserved for backward compatibility and special usecases (like changing the number of vCPUs). In addition, it becomes able to simplify tests a lot. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
The previous commit made `--config` optional, but it is used to specify a CPU template to verify. To allow users to verify a CPU template without passing a Firecracker config file, adds `--template` to the command. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Now that there is no need to specify `--config` to `cpu-template-helper`, removes `--config` from integration tests and simplifies them. Also regathers fingerprint files. All the fingerprint changes are caused by the difference of the number of vCPUs: the Firecracker's default value is 1 vCPU but the test config value is 2 vCPUs. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Previously, only consecutive guest CPU configs were verified. A fingerprint contains not only guest CPU config but also host configuration. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
zulinx86
force-pushed
the
improve_helper
branch
from
d56f643 to
e370111
Compare
ShadowCurse
approved these changes
Mar 15, 2024
pb8o
approved these changes
Mar 15, 2024
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.
Reasons
This PR mainly aims to enhance the usability of the CPU template helper tool.
The most inconvenient thing is that users need to prepare kernel, rootfs and Firecracker config to run the tool, although it doesn't actually start the microVM and those resources don't lead to meaningful changes to guest CPU config. But unfortunately, it cannot run without those resources because the CPU template helper tool should use the Firecracker's microVM initialization process (
build_microvm_for_boot()ofvmmcrate) which requires such resources.Changes
The main change is: CPU template helper tool prepares some required resources (kernel, rootfs and Firecracker config) on the behalf of users if
--configparameter is not provided.There are some refactoring commits in the first 6 commits.
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
[ ] If a specific issue led to this PR, this PR closes the issue.PR.
[ ] API changes follow the Runbook for Firecracker API changes.CHANGELOG.md.TODOs link to an issue.contribution quality standards.
rust-vmm.