-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: reset KVM_REG_ARM_PTIMER_CNT on VM boot #4987
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
Merged
ShadowCurse
merged 5 commits into
firecracker-microvm:main
from
ShadowCurse:aarch64_counter_reset
Jan 17, 2025
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
4bca17f
refactor: move KVM related logic into a separate struct
ShadowCurse 87f7830
feat: reset KVM_REG_ARM_PTIMER_CNT on VM boot
ShadowCurse b29ab67
chore: update prod-host-setup.md with arm physical counter info
ShadowCurse a9a7e68
chore: add an entry to the CHANGELOG
ShadowCurse d59728f
test: add integration test for physical counter reset
ShadowCurse File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -328,13 +328,16 @@ For vendor-specific recommendations, please consult the resources below: | |
| - ARM: | ||
| [Speculative Processor Vulnerability](https://developer.arm.com/support/arm-security-updates/speculative-processor-vulnerability) | ||
|
|
||
| ##### [ARM only] Physical counter directly passed through to the guest | ||
| ##### [ARM only] VM Physical counter behaviour | ||
|
|
||
| On ARM, the physical counter (i.e `CNTPCT`) it is returning the | ||
| [actual EL1 physical counter value of the host][1]. From the discussions before | ||
| merging this change [upstream][2], this seems like a conscious design decision | ||
| of the ARM code contributors, giving precedence to performance over the ability | ||
| to trap and control this in the hypervisor. | ||
| On ARM, Firecracker tries to reset the `CNTPCT` physical counter on VM boot. | ||
| This is done in order to prevent VM from reading host physical counter value. | ||
| Firecracker will only try to reset the counter if the host KVM contains | ||
| `KVM_CAP_COUNTER_OFFSET` capability. This capability is only present in kernels | ||
| containing | ||
| [this](https://lore.kernel.org/all/[email protected]/) | ||
| patch series (starting from 6.4 and newer). For older kernels the counter value | ||
| will be passed through from the host. | ||
|
|
||
| ##### Verification | ||
|
|
||
|
|
@@ -428,6 +431,3 @@ To validate that the change took effect, the file | |
| [^1]: Look for `GRUB_CMDLINE_LINUX` in file `/etc/default/grub` in RPM-based | ||
| systems, and | ||
| [this doc for Ubuntu](https://wiki.ubuntu.com/Kernel/KernelBootParameters). | ||
|
|
||
| [1]: https://elixir.free-electrons.com/linux/v4.14.203/source/virt/kvm/arm/hyp/timer-sr.c#L63 | ||
| [2]: https://lists.cs.columbia.edu/pipermail/kvmarm/2017-January/023323.html | ||
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ | |
|
|
||
| use super::get_fdt_addr; | ||
| use super::regs::*; | ||
| use crate::vstate::kvm::OptionalCapabilities; | ||
| use crate::vstate::memory::GuestMemoryMmap; | ||
|
|
||
| /// Errors thrown while setting aarch64 registers. | ||
|
|
@@ -78,6 +79,7 @@ | |
| cpu_id: u8, | ||
| boot_ip: u64, | ||
| mem: &GuestMemoryMmap, | ||
| optional_capabilities: &OptionalCapabilities, | ||
| ) -> Result<(), VcpuError> { | ||
| let kreg_off = offset_of!(kvm_regs, regs); | ||
|
|
||
|
|
@@ -106,6 +108,23 @@ | |
| vcpufd | ||
| .set_one_reg(id, &get_fdt_addr(mem).to_le_bytes()) | ||
| .map_err(|err| VcpuError::SetOneReg(id, err))?; | ||
|
|
||
| // Reset the physical counter for the guest. This way we avoid guest reading | ||
| // host physical counter. | ||
| // Resetting KVM_REG_ARM_PTIMER_CNT for single vcpu is enough because there is only | ||
| // one timer struct with offsets per VM. | ||
| // Because the access to KVM_REG_ARM_PTIMER_CNT is only present starting 6.4 kernel, | ||
ShadowCurse marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // we only do the reset if KVM_CAP_COUNTER_OFFSET is present as it was added | ||
| // in the same patch series as the ability to set the KVM_REG_ARM_PTIMER_CNT register. | ||
| // Path series which introduced the needed changes: | ||
| // https://lore.kernel.org/all/[email protected]/ | ||
| // Note: the value observed by the guest will still be above 0, because there is a delta | ||
| // time between this resetting and first call to KVM_RUN. | ||
| if optional_capabilities.counter_offset { | ||
| vcpufd | ||
| .set_one_reg(KVM_REG_ARM_PTIMER_CNT, &[0; 8]) | ||
| .map_err(|err| VcpuError::SetOneReg(id, err))?; | ||
| } | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
@@ -214,20 +233,21 @@ | |
| #[cfg(test)] | ||
| mod tests { | ||
| #![allow(clippy::undocumented_unsafe_blocks)] | ||
| use kvm_ioctls::Kvm; | ||
|
|
||
| use super::*; | ||
| use crate::arch::aarch64::layout; | ||
| use crate::test_utils::arch_mem; | ||
| use crate::vstate::kvm::Kvm; | ||
|
|
||
| #[test] | ||
| fn test_setup_regs() { | ||
| let kvm = Kvm::new().unwrap(); | ||
| let vm = kvm.create_vm().unwrap(); | ||
| let kvm = Kvm::new(vec![]).unwrap(); | ||
| let vm = kvm.fd.create_vm().unwrap(); | ||
| let vcpu = vm.create_vcpu(0).unwrap(); | ||
| let mem = arch_mem(layout::FDT_MAX_SIZE + 0x1000); | ||
| let optional_capabilities = kvm.optional_capabilities(); | ||
|
|
||
| let res = setup_boot_regs(&vcpu, 0, 0x0, &mem); | ||
| let res = setup_boot_regs(&vcpu, 0, 0x0, &mem, &optional_capabilities); | ||
| assert!(matches!( | ||
| res.unwrap_err(), | ||
| VcpuError::SetOneReg(0x6030000000100042, _) | ||
|
|
@@ -237,13 +257,31 @@ | |
| vm.get_preferred_target(&mut kvi).unwrap(); | ||
| vcpu.vcpu_init(&kvi).unwrap(); | ||
|
|
||
| setup_boot_regs(&vcpu, 0, 0x0, &mem).unwrap(); | ||
| setup_boot_regs(&vcpu, 0, 0x0, &mem, &optional_capabilities).unwrap(); | ||
|
|
||
| // Check that the register is reset on compatible kernels. | ||
ShadowCurse marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Because there is a delta in time between we reset the register and time we | ||
| // read it, we cannot compare with 0. Instead we compare it with meaningfully | ||
| // small value. | ||
| if optional_capabilities.counter_offset { | ||
| let mut reg_bytes = [0_u8; 8]; | ||
| vcpu.get_one_reg(SYS_CNTPCT_EL0, &mut reg_bytes).unwrap(); | ||
| let counter_value = u64::from_le_bytes(reg_bytes); | ||
|
|
||
| // We are reading the SYS_CNTPCT_EL0 right after resetting it. | ||
| // If reset did happen successfully, the value should be quite small when we read it. | ||
| // If the reset did not happen, the value will be same as on the host and it surely | ||
| // will be more that MAX_VALUE. | ||
| let max_value = 1000; | ||
|
|
||
| assert!(counter_value < max_value); | ||
| } | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_read_mpidr() { | ||
| let kvm = Kvm::new().unwrap(); | ||
| let vm = kvm.create_vm().unwrap(); | ||
| let kvm = Kvm::new(vec![]).unwrap(); | ||
| let vm = kvm.fd.create_vm().unwrap(); | ||
| let vcpu = vm.create_vcpu(0).unwrap(); | ||
| let mut kvi: kvm_bindings::kvm_vcpu_init = kvm_bindings::kvm_vcpu_init::default(); | ||
| vm.get_preferred_target(&mut kvi).unwrap(); | ||
|
|
@@ -261,8 +299,8 @@ | |
|
|
||
| #[test] | ||
| fn test_get_set_regs() { | ||
| let kvm = Kvm::new().unwrap(); | ||
| let vm = kvm.create_vm().unwrap(); | ||
| let kvm = Kvm::new(vec![]).unwrap(); | ||
| let vm = kvm.fd.create_vm().unwrap(); | ||
| let vcpu = vm.create_vcpu(0).unwrap(); | ||
| let mut kvi: kvm_bindings::kvm_vcpu_init = kvm_bindings::kvm_vcpu_init::default(); | ||
| vm.get_preferred_target(&mut kvi).unwrap(); | ||
|
|
@@ -283,8 +321,8 @@ | |
| fn test_mpstate() { | ||
| use std::os::unix::io::AsRawFd; | ||
|
|
||
| let kvm = Kvm::new().unwrap(); | ||
| let vm = kvm.create_vm().unwrap(); | ||
| let kvm = Kvm::new(vec![]).unwrap(); | ||
| let vm = kvm.fd.create_vm().unwrap(); | ||
| let vcpu = vm.create_vcpu(0).unwrap(); | ||
| let mut kvi: kvm_bindings::kvm_vcpu_init = kvm_bindings::kvm_vcpu_init::default(); | ||
| vm.get_preferred_target(&mut kvi).unwrap(); | ||
|
|
||
This file contains hidden or 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
This file contains hidden or 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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.