Refactoring + Unifying CPU template handling #3610
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
force-pushed
the
feature/cpu-templates
branch
from
33fe3db
to
2917a72
Compare
Update error items to use upper camel case for consistency. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
zulinx86
force-pushed
the
refactor
branch
26 times, most recently
from
f8041bb
to
20b6429
Compare
Just move static CPU templates defined with internal data structures used in custom CPU template into a temporal directory inside vmm crate with no functional changes, in order to wire up them in successive commits. The moved static CPU templates were added in PR firecracker-microvm#3598 in static_templates crate temporarily to check that they are compatible with static CPU templates in the main branch. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Wire static_cpu_templates_new module up to vmm crate. The module was moved in the directory in the previous commit, but had not yet to be wired up. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Add and implement `GetCpuTemplate` trait for x86_64 and aarch64 to get the wrapped internal `CustomCpuTemplate` from `Option<CpuTemplateType>`. The implemented trait has not been used in this commit, but will be consumed in successive commits to refactor CPU template application. Signed-off-by: Takahiro Itazuri <itazur@amazon.com> Co-authored-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Move the code to get MSR values into `KvmVcpu::get_msrs()`. This method will be consumed in multiple places by successive commits. Signed-off-by: Takahiro Itazuri <itazur@amazon.com> Co-authored-by: Nikita Kalyazin <kalyazin@amazon.com>
zulinx86
force-pushed
the
refactor
branch
6 times, most recently
from
5211eea
to
bae04e4
Compare
kalyazin
previously approved these changes
Apr 18, 2023
Check whether the vendor is matched between the static CPU template and the actual CPU. Static CPU templates for x86-64 are specific to vendor (Intel or AMD). `include_leaves_from()` method does this check now, but will be removed in a successive commit. Instead of this method, we do this check in `get_cpu_template()`. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Build a custom `CpuConfiguration` with 2 steps: 1. Construct a base `CpuConfiguration`. 2. Apply CPU template to the default `CpuConfiguration`. Previously, all these processes were done in `CpuConfiguration::new()`, but this approach is a bit complex in the test perspective. In addition, we will be able to unify `CpuConfiguration` interface between x86_64 and aarch64 and simplify `configure_system_for_boot()` function in successive commits. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Add `KvmVcpu::get_regs()` to retrieve values for the given register IDs through `KVM_GET_ONE_REG`. This will be consumed in a successive commit. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
As done in x86_64, build a custom `CpuConfiguration` with 2 steps: 1. Construct a base `CpuConfiguration`. 2. Apply CPU template to the default `CpuConfiguration`. By providing the same interface between x86_64 and aarch64, it becomes possible to unify some operations. Signed-off-by: Takahiro Itazuri <itazur@amazon.com> Co-authored-by: Egor Lazarchuk <yegorlz@amazon.co.uk>
Move out arch-agnostic operations from arch-specific operations. Thanks to the previous commits, now it is possible to do this. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Replace the static CPU templates introduced in PR firecracker-microvm#3076 with ones that are introduced in PR firecracker-microvm#3598, are compatibile with main branch and are defined using `CustomCpuTemplate`. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Remove include_leaves_from() that is no longer used. Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
The CPU model check of T2CL template was added in PR firecracker-microvm#3480, but was removed in commit 8f5feb0 ("feat: Updated CPUID"). Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
Signed-off-by: Takahiro Itazuri <itazur@amazon.com>
mattschlebusch
approved these changes
Apr 18, 2023
kalyazin
approved these changes
Apr 18, 2023
4 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.
Major Changes
For more details, please see the commit messages!
Reason
Previously static and custom CPU templates are handled in different ways and
CpuConfigurationprovides different interfaces between architectures. Unifying them as much as possible introduces simplicity, resulting in maintainability.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.[ ] Any required documentation changes (code and docs) are included in this PR.[ ] API changes follow the Runbook for Firecracker API changes.[ ] User-facing changes are mentioned inCHANGELOG.md.[ ] NewTODOs link to an issue.[ ] This functionality cannot be added inrust-vmm.