From: Amirmohammad Eftekhar When computing the TSC multiplier for L2, use the minimum/maximum frequency supported by hardware if the resulting multiplier can't be programmed into hardware, i.e. saturate L2's frequency on both sides. This can happen if userspace is running L1 at a low/high frequency relative to L0, and L1 is doing the same for L2 (relative to L1), because although each of the L1 and L2 inputs are constrained and validated, the end result can exceed SVM's and VMX's architectural limits. Don't bother trying to log an error or do something more sophisticated, because in practice only a misbehaving L1 (and/or host userspace) will run afoul of the flaw. On SVM, which has the smallest "range" by far (8 bits for the integer multiplier, versus 16 bits on VMX), KVM can still run L2 with a frequency 255x that of L0. Even assuming a pessimistic L0 TSC frequency of 1GHz (modern CPUs run TSC at 2GHz+), L2 would need to be running at a whopping ~255GHz for the limitation to be a problem. Not to mention that if L2 is running at a legitimate frequency, then running that VM on existing hardware is already doomed irrespective of the nested angle. Open code the mul_u64_u64_shr() if the compiler natively supports 128-bit integers, mostly to avoid having to do the multiply twice for what should be a very rare scenario, but also so that there's a slightly more scrutable sequence that can be read to understand what all is going on. Signed-off-by: Amirmohammad Eftekhar [sean: optimize for INT128, rewrite comment+changelog to be less AI] Signed-off-by: Sean Christopherson --- arch/x86/kvm/x86.c | 55 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index cf3fcdfd8ad2..037c7ea5c5a7 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -1190,11 +1190,58 @@ EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_calc_nested_tsc_offset); u64 kvm_calc_nested_tsc_multiplier(u64 l1_multiplier, u64 l2_multiplier) { - if (l2_multiplier != kvm_caps.default_tsc_scaling_ratio) - return mul_u64_u64_shr(l1_multiplier, l2_multiplier, - kvm_caps.tsc_scaling_ratio_frac_bits); + u8 frac_bits = kvm_caps.tsc_scaling_ratio_frac_bits; + u64 nested_multiplier; - return l1_multiplier; + if (l2_multiplier == kvm_caps.default_tsc_scaling_ratio) + return l1_multiplier; + + /* + * The shift is fixed on both AMD and Intel, and operates on a 64-bit + * value. I.e. a shift greater than 63 is completely nonsensical. + */ + if (WARN_ON_ONCE(frac_bits > 63)) + return l1_multiplier; + + /* + * If the resulting multiplier can't be programmed into hardware, run + * L2 at the minimum/maximum frequency supported by hardware, i.e. + * saturate L2's frequency on both sides. Because L2's frequency needs + * to be distilled down to a single multiplier to get from: + * + * L2 = (((L0 * L1_mult) >> frac) * L2_mult) >> frac) + * + * to: + * + * L2 = (L0 * mult) >> frac + * + * very small/large L1 and L2 multipliers can underflow/overflow the + * minimum/maximum multiplier supported by hardware when combined into + * a single value. + * + * Manually check for the case where the result would overflow a u64, + * i.e. if the multiplier would be silently truncated before the "too + * large" check. Avoid doing the multiply twice in the common case + * where the compiler natively supports 128-bit values. + */ +#ifdef CONFIG_ARCH_SUPPORTS_INT128 + unsigned __int128 m = (unsigned __int128)l1_multiplier * l2_multiplier; + + if (m >> (64 + frac_bits)) + return kvm_caps.max_tsc_scaling_ratio; + + nested_multiplier = m >> frac_bits; +#else + if (mul_u64_u64_shr(l1_multiplier, l2_multiplier, 64 + frac_bits)) + return kvm_caps.max_tsc_scaling_ratio; + + nested_multiplier = mul_u64_u64_shr(l1_multiplier, l2_multiplier, frac_bits); +#endif + if (nested_multiplier > kvm_caps.max_tsc_scaling_ratio) + return kvm_caps.max_tsc_scaling_ratio; + + /* The minimum multiplier is '1' on both AMD and Intel. */ + return nested_multiplier ?: 1; } EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_calc_nested_tsc_multiplier); -- 2.56.0.rc1.315.gc6ed9934b7-goog Explicitly WARN and fallback to the default TSC ratio, i.e. run the guest at L0's TSC frequency, if KVM tries to program a bad multiplier value. As pointed out by Sashiko, leaving the MSR as-is bleeds state from the vCPU that last ran on the pCPU into the likely-misbehaving current vCPU. Leaking a vCPU's frequency isn't very interesting, and corrupting KVM's cache isn't a big deal either since KVM would only refuse to try to write the same bad value in the future, but falling back to the default value is trivial, and explicitly WARNing ensures a KVM bug won't slip by silently. E.g. because ex_handler_msr() only WARNs once for *all* WRMSRs, it's possible for the potentially-fatal-to-the-guest issue to not exhibit any visible symptoms in the host. Reported-by: Sashiko Bot Closes: https://lore.kernel.org/all/20260722091414.E842B1F000E9@smtp.kernel.org Signed-off-by: Sean Christopherson --- arch/x86/kvm/svm/svm.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c index 0eb1623052c1..2b6888417bc0 100644 --- a/arch/x86/kvm/svm/svm.c +++ b/arch/x86/kvm/svm/svm.c @@ -533,6 +533,15 @@ static int svm_check_processor_compat(void) static void __svm_write_tsc_multiplier(u64 multiplier) { + /* + * Fallback to the default ratio if KVM is buggy and tries to program + * an unsupported scaling ratio, e.g. so that the guest has a chance of + * surviving, so that the cache isn't stale/corrupted, and so that KVM + * doesn't leak state across VMs. + */ + if (WARN_ON_ONCE(multiplier & SVM_TSC_RATIO_RSVD)) + multiplier = SVM_TSC_RATIO_DEFAULT; + if (multiplier == __this_cpu_read(current_tsc_ratio)) return; -- 2.56.0.rc1.315.gc6ed9934b7-goog Reject KVM_SET_TSC_KHZ if the incoming frequency is strictly greater than KVM's max supported frequency, not if the frequency is greater than *or* equal to the max frequency. The effective off-by-one bug came about via (dubious) review feedback, which also subtly collided with a functional change in later versions of the original TSC scaling series. In v2 of the original TSC scaling series[1], KVM set its absolute min/max to [1, UINT_MAX], i.e. allowed any value that would fit in the u32 passed to KVM_SET_TSC_KHZ[2]. min = max(1ULL, __scale_tsc(tsc_khz, TSC_RATIO_MIN)); max = min(0xffffffffULL, __scale_tsc(tsc_khz, TSC_RATIO_MAX)); That prompted Avi to suggest rejecting the "equals" case, presumably because the check would always succeed given an absolute max of UINT_MAX? But even that doesn't hold up to scrutiny, as allowing the actual min/max isn't inherently unsafe. Regardless, that feedback was taken and applied to future versions, but only for the maximum, not the minimum. > + > + r = -EINVAL; > + if (user_tsc_khz< kvm_min_guest_tsc_khz || > + user_tsc_khz> kvm_max_guest_tsc_khz) <= and >= are probably safer. v3 of the series[3] then also realized that allowing UINT_MAX would lead to undesirable interactions with KVM_GET_TSC_KHZ, which returns a *signed* 32-bit integer that is implicitly converted into a signed 64-bit value on 64-bit kernels. I.e. allowing a value greater than INT_MAX would result in userspace observing a negative value when doing KVM_GET_TSC_KHZ after KVM_SET_TSC_KHZ. /* * Make sure the user can only configure tsc_khz values that * fit into a signed integer. * A min value is not calculated needed because it will always * be 1 on all machines and a value of 0 is used to disable * tsc-scaling for the vcpu. */ max = min(0x7fffffffULL, __scale_tsc(tsc_khz, TSC_RATIO_MAX)); kvm_max_guest_tsc_khz = max; v3 also dropped the explicit minimum tracking, as both AMD and Intel support a minimum *fractional* ratio of 1, i.e. AMD and Intel support a minimum frequency of "host / 2^32" and "host / 2^48" respectively. And because KVM_GET_TSC_KHZ (and KVM itself) only supports frequencies that fit in a signed 32-bit integer, even AMD's more coarse-grained ratio can scale down any host frequency to '1', i.e. KVM can always support a minimum frequency of 1KHz. Rather than explicitly reject a frequency of 1KHz, v3 simply dropped the minimum check, i.e. ignored the "<=" suggestion, but kept the ">=" side of things. As a result, KVM now has a bizarre uABI where KVM_SET_TSC_KHZ tops out at INT_MAX-1 for no discernible reason. Fix the off-by-one flaw to provide a less weird uABI, and so that KVM_SET_TSC_KHZ accepts the maximum possible value that can be returned by KVM_GET_TSC_KHZ (without running afoul of casting issues; KVM doesn't actually sanity check that tsc_khz fits in a signed 32-bit value, which is a non-issue in practice because the units are KHz, not Hz, i.e. KVM_GET_TSC_KHZ is fine until CPUs with a TSC frequency greater than ~2.147PHz come along). Note, in practice, no real world VMM is likely to care. As above, running afoul of the off-by-one issue would mean trying to configure a virtual TSC frequency that is three orders of magnitude greater than what current CPUs support. Opportunistically explain *why* KVM restricts KVM_SET_TSC_KHZ to values that fit in a signed integer, as it requires far too much spelunking to piece together the connection to KVM_GET_TSC_KHZ. Link: https://lore.kernel.org/all/1300952424-32014-1-git-send-email-joerg.roedel@amd.com [1] Link: https://lore.kernel.org/all/1300952424-32014-7-git-send-email-joerg.roedel@amd.com [2] Link: https://lore.kernel.org/all/1301042691-22929-7-git-send-email-joerg.roedel@amd.com [3] Signed-off-by: Sean Christopherson --- arch/x86/kvm/x86.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 037c7ea5c5a7..1d25ffcf9273 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3718,7 +3718,7 @@ long kvm_arch_vcpu_ioctl(struct file *filp, user_tsc_khz = (u32)arg; if (kvm_caps.has_tsc_control && - user_tsc_khz >= kvm_caps.max_guest_tsc_khz) + user_tsc_khz > kvm_caps.max_guest_tsc_khz) goto out; if (user_tsc_khz == 0) @@ -4685,7 +4685,7 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg) user_tsc_khz = (u32)arg; if (kvm_caps.has_tsc_control && - user_tsc_khz >= kvm_caps.max_guest_tsc_khz) + user_tsc_khz > kvm_caps.max_guest_tsc_khz) goto out; if (user_tsc_khz == 0) @@ -7174,7 +7174,8 @@ int kvm_x86_vendor_init(struct kvm_x86_init_ops *ops) if (kvm_caps.has_tsc_control) { /* * Make sure the user can only configure tsc_khz values that - * fit into a signed integer. + * fit into a signed integer, otherwise KVM_GET_TSC_KHZ would + * return a negative value and confuse userspace. * A min value is not calculated because it will always * be 1 on all machines. */ -- 2.56.0.rc1.315.gc6ed9934b7-goog Use KVM's pRNG to randomize L1's TSC ratio in the nested TSC scaling test, so that reproducing a failure with a specific seed is slightly easier. Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index c763f4cf0f62..65de46f80a3e 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -166,8 +166,7 @@ int main(int argc, char *argv[]) * referenced by both main() and l1_guest_code() and using a global * variable does not work. */ - srand(time(NULL)); - l1_scale_factor = (rand() % 9) + 2; + l1_scale_factor = (kvm_random_u32(&kvm_rng) % 9) + 2; printf("L1's scale down factor is: %"PRIu64"\n", l1_scale_factor); printf("L2's scale up factor is: %llu\n", L2_SCALE_FACTOR); -- 2.56.0.rc1.315.gc6ed9934b7-goog Drop a redundant VMWRITE of TSC_MULTIPLIER_HIGH in the nested TSC scaling test. KVM selftests are 64-bit only, i.e. the full 64-bit multiplier is always written by setting TSC_MULTIPLIER. Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index 65de46f80a3e..052ddd61482d 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -122,7 +122,6 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages) vmwrite(TSC_OFFSET, TSC_OFFSET_L2); vmwrite(TSC_MULTIPLIER, TSC_MULTIPLIER_L2); - vmwrite(TSC_MULTIPLIER_HIGH, TSC_MULTIPLIER_L2 >> 32); /* launch L2 */ vmlaunch(); -- 2.56.0.rc1.315.gc6ed9934b7-goog Use a plain "lu" instead of "PRIu64" to print u64s in the nested TSC scaling test, as KVM selftests are 64-bit only (and supporting 32-bit x86 selftest would be an absurdly large lift, for something absolutely no one cares about). Signed-off-by: Sean Christopherson --- .../kvm/x86/nested_tsc_scaling_test.c | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index 052ddd61482d..b8b2217b9efa 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -41,14 +41,9 @@ static void compare_tsc_freq(u64 actual, u64 expected) thresh_low = expected - tolerance; thresh_high = expected + tolerance; - TEST_ASSERT(thresh_low < actual, - "TSC freq is expected to be between %"PRIu64" and %"PRIu64 - " but it actually is %"PRIu64, - thresh_low, thresh_high, actual); - TEST_ASSERT(thresh_high > actual, - "TSC freq is expected to be between %"PRIu64" and %"PRIu64 - " but it actually is %"PRIu64, - thresh_low, thresh_high, actual); + TEST_ASSERT(thresh_low < actual && thresh_high > actual, + "TSC freq is '%lu', expected to be between %lu and %lu", + actual, thresh_low, thresh_high); } static void check_tsc_freq(int level) @@ -166,7 +161,7 @@ int main(int argc, char *argv[]) * variable does not work. */ l1_scale_factor = (kvm_random_u32(&kvm_rng) % 9) + 2; - printf("L1's scale down factor is: %"PRIu64"\n", l1_scale_factor); + printf("L1's scale down factor is: %lu\n", l1_scale_factor); printf("L2's scale up factor is: %llu\n", L2_SCALE_FACTOR); tsc_start = rdtsc(); @@ -174,7 +169,7 @@ int main(int argc, char *argv[]) tsc_end = rdtsc(); l0_tsc_freq = tsc_end - tsc_start; - printf("real TSC frequency is around: %"PRIu64"\n", l0_tsc_freq); + printf("real TSC frequency is around: %lu\n", l0_tsc_freq); vm = vm_create_with_one_vcpu(&vcpu, l1_guest_code); @@ -207,16 +202,14 @@ int main(int argc, char *argv[]) break; case UCHECK_L1: l1_tsc_freq = uc.args[1]; - printf("L1's TSC frequency is around: %"PRIu64 - "\n", l1_tsc_freq); + printf("L1's TSC frequency is around: %lu\n", l1_tsc_freq); compare_tsc_freq(l1_tsc_freq, l0_tsc_freq / l1_scale_factor); break; case UCHECK_L2: l2_tsc_freq = uc.args[1]; - printf("L2's TSC frequency is around: %"PRIu64 - "\n", l2_tsc_freq); + printf("L2's TSC frequency is around: %lu\n", l2_tsc_freq); compare_tsc_freq(l2_tsc_freq, l1_tsc_freq * L2_SCALE_FACTOR); -- 2.56.0.rc1.315.gc6ed9934b7-goog Randomize L2's scale factor in the nested TSC scaling test; the test's comment that "using a global variable does not work" just means the author didn't know how to use sync_global_to_guest(). Signed-off-by: Sean Christopherson --- .../kvm/x86/nested_tsc_scaling_test.c | 21 ++++++++----------- 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index b8b2217b9efa..84312146e93f 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -17,10 +17,9 @@ #include "kselftest.h" /* L2 is scaled up (from L1's perspective) by this factor */ -#define L2_SCALE_FACTOR 4ULL +static u64 l2_scale_factor; #define TSC_OFFSET_L2 ((u64)-33125236320908) -#define TSC_MULTIPLIER_L2 (L2_SCALE_FACTOR << 48) enum { USLEEP, UCHECK_L1, UCHECK_L2 }; #define GUEST_SLEEP(sec) ucall(UCALL_SYNC, 2, USLEEP, sec) @@ -81,7 +80,7 @@ static void l1_svm_code(struct svm_test_data *svm) generic_svm_setup(svm, l2_guest_code); /* enable TSC scaling for L2 */ - wrmsr(MSR_AMD64_TSC_RATIO, L2_SCALE_FACTOR << 32); + wrmsr(MSR_AMD64_TSC_RATIO, l2_scale_factor << 32); /* launch L2 */ run_guest(svm->vmcb, svm->vmcb_gpa); @@ -116,7 +115,7 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages) vmwrite(SECONDARY_VM_EXEC_CONTROL, control); vmwrite(TSC_OFFSET, TSC_OFFSET_L2); - vmwrite(TSC_MULTIPLIER, TSC_MULTIPLIER_L2); + vmwrite(TSC_MULTIPLIER, l2_scale_factor << 48); /* launch L2 */ vmlaunch(); @@ -154,15 +153,12 @@ int main(int argc, char *argv[]) TEST_REQUIRE(kvm_has_cap(KVM_CAP_TSC_CONTROL)); TEST_REQUIRE(sys_clocksource_is_based_on_tsc()); - /* - * We set L1's scale factor to be a random number from 2 to 10. - * Ideally we would do the same for L2's factor but that one is - * referenced by both main() and l1_guest_code() and using a global - * variable does not work. - */ + /* Scale L1 "down" and L2 "up" at a random factor from 2 to 10. */ l1_scale_factor = (kvm_random_u32(&kvm_rng) % 9) + 2; printf("L1's scale down factor is: %lu\n", l1_scale_factor); - printf("L2's scale up factor is: %llu\n", L2_SCALE_FACTOR); + + l2_scale_factor = (kvm_random_u32(&kvm_rng) % 9) + 2; + printf("L2's scale up factor is: %lu\n", l2_scale_factor); tsc_start = rdtsc(); sleep(1); @@ -172,6 +168,7 @@ int main(int argc, char *argv[]) printf("real TSC frequency is around: %lu\n", l0_tsc_freq); vm = vm_create_with_one_vcpu(&vcpu, l1_guest_code); + sync_global_to_guest(vm, l2_scale_factor); if (kvm_cpu_has(X86_FEATURE_VMX)) vcpu_alloc_vmx(vm, &guest_gva); @@ -212,7 +209,7 @@ int main(int argc, char *argv[]) printf("L2's TSC frequency is around: %lu\n", l2_tsc_freq); compare_tsc_freq(l2_tsc_freq, - l1_tsc_freq * L2_SCALE_FACTOR); + l1_tsc_freq * l2_scale_factor); break; } break; -- 2.56.0.rc1.315.gc6ed9934b7-goog Rename the nested TSC scaling test's helpers to clarify which one is used by guest code and which one is used by host code, and to make it more obvious that the host version *checks* the result and asserts on errors. Contrary to what "compare" suggests, the helper doesn't return whether or not the actual vs. expected values match. Signed-off-by: Sean Christopherson --- .../kvm/x86/nested_tsc_scaling_test.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index 84312146e93f..e5fc4c9bb9f9 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -32,7 +32,7 @@ enum { USLEEP, UCHECK_L1, UCHECK_L2 }; * measurements, a difference of 1% between the actual and the expected value * is tolerated. */ -static void compare_tsc_freq(u64 actual, u64 expected) +static void host_check_tsc_freq(u64 actual, u64 expected) { u64 tolerance, thresh_low, thresh_high; @@ -45,7 +45,7 @@ static void compare_tsc_freq(u64 actual, u64 expected) actual, thresh_low, thresh_high); } -static void check_tsc_freq(int level) +static void guest_check_tsc_freq(int level) { u64 tsc_start, tsc_end, tsc_freq; @@ -66,7 +66,7 @@ static void check_tsc_freq(int level) static void l2_guest_code(void) { - check_tsc_freq(UCHECK_L2); + guest_check_tsc_freq(UCHECK_L2); /* exit to L1 */ __asm__ __volatile__("vmcall"); @@ -75,7 +75,7 @@ static void l2_guest_code(void) static void l1_svm_code(struct svm_test_data *svm) { /* check that L1's frequency looks alright before launching L2 */ - check_tsc_freq(UCHECK_L1); + guest_check_tsc_freq(UCHECK_L1); generic_svm_setup(svm, l2_guest_code); @@ -87,7 +87,7 @@ static void l1_svm_code(struct svm_test_data *svm) GUEST_ASSERT(svm->vmcb->control.exit_code == SVM_EXIT_VMMCALL); /* check that L1's frequency still looks good */ - check_tsc_freq(UCHECK_L1); + guest_check_tsc_freq(UCHECK_L1); GUEST_DONE(); } @@ -97,7 +97,7 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages) u32 control; /* check that L1's frequency looks alright before launching L2 */ - check_tsc_freq(UCHECK_L1); + guest_check_tsc_freq(UCHECK_L1); prepare_for_vmx_operation(vmx_pages); load_vmcs(vmx_pages); @@ -122,7 +122,7 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages) GUEST_ASSERT(vmread(VM_EXIT_REASON) == EXIT_REASON_VMCALL); /* check that L1's frequency still looks good */ - check_tsc_freq(UCHECK_L1); + guest_check_tsc_freq(UCHECK_L1); GUEST_DONE(); } @@ -201,14 +201,14 @@ int main(int argc, char *argv[]) l1_tsc_freq = uc.args[1]; printf("L1's TSC frequency is around: %lu\n", l1_tsc_freq); - compare_tsc_freq(l1_tsc_freq, + host_check_tsc_freq(l1_tsc_freq, l0_tsc_freq / l1_scale_factor); break; case UCHECK_L2: l2_tsc_freq = uc.args[1]; printf("L2's TSC frequency is around: %lu\n", l2_tsc_freq); - compare_tsc_freq(l2_tsc_freq, + host_check_tsc_freq(l2_tsc_freq, l1_tsc_freq * l2_scale_factor); break; } -- 2.56.0.rc1.315.gc6ed9934b7-goog Extract the guts of the nested TSC scaling test to a separate helper in anticipation of extending the test beyond its single "scale L1 up, L2 down" testcase. No functional change intended. Signed-off-by: Sean Christopherson --- .../kvm/x86/nested_tsc_scaling_test.c | 54 ++++++++++--------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index e5fc4c9bb9f9..d3208b57eafc 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -135,38 +135,18 @@ static void l1_guest_code(void *data) l1_svm_code(data); } -int main(int argc, char *argv[]) +static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale) { + u64 tsc_khz, l1_tsc_freq, l2_tsc_freq; struct kvm_vcpu *vcpu; struct kvm_vm *vm; - gva_t guest_gva = 0; + gva_t guest_gva; - u64 tsc_start, tsc_end; - u64 tsc_khz; - u64 l1_scale_factor; - u64 l0_tsc_freq = 0; - u64 l1_tsc_freq = 0; - u64 l2_tsc_freq = 0; + l2_scale_factor = l2_scale; - TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_VMX) || - kvm_cpu_has(X86_FEATURE_SVM)); - TEST_REQUIRE(kvm_has_cap(KVM_CAP_TSC_CONTROL)); - TEST_REQUIRE(sys_clocksource_is_based_on_tsc()); - - /* Scale L1 "down" and L2 "up" at a random factor from 2 to 10. */ - l1_scale_factor = (kvm_random_u32(&kvm_rng) % 9) + 2; printf("L1's scale down factor is: %lu\n", l1_scale_factor); - - l2_scale_factor = (kvm_random_u32(&kvm_rng) % 9) + 2; printf("L2's scale up factor is: %lu\n", l2_scale_factor); - tsc_start = rdtsc(); - sleep(1); - tsc_end = rdtsc(); - - l0_tsc_freq = tsc_end - tsc_start; - printf("real TSC frequency is around: %lu\n", l0_tsc_freq); - vm = vm_create_with_one_vcpu(&vcpu, l1_guest_code); sync_global_to_guest(vm, l2_scale_factor); @@ -183,6 +163,9 @@ int main(int argc, char *argv[]) /* scale down L1's TSC frequency */ vcpu_ioctl(vcpu, KVM_SET_TSC_KHZ, (void *) (tsc_khz / l1_scale_factor)); + /* L1 will communicate its frequency before the L2 check.*/ + l1_tsc_freq = 0; + for (;;) { struct ucall uc; @@ -222,5 +205,28 @@ int main(int argc, char *argv[]) done: kvm_vm_free(vm); +} + +int main(int argc, char *argv[]) +{ + u64 l0_tsc_freq, tsc_start, tsc_end, l1_scale, l2_scale; + + TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_VMX) || + kvm_cpu_has(X86_FEATURE_SVM)); + TEST_REQUIRE(kvm_has_cap(KVM_CAP_TSC_CONTROL)); + TEST_REQUIRE(sys_clocksource_is_based_on_tsc()); + + tsc_start = rdtsc(); + sleep(1); + tsc_end = rdtsc(); + + l0_tsc_freq = tsc_end - tsc_start; + printf("real TSC frequency is around: %lu\n", l0_tsc_freq); + + /* Scale L1 "down" and L2 "up" at a random factor from 2 to 10. */ + l1_scale = (kvm_random_u32(&kvm_rng) % 9) + 2; + l2_scale = (kvm_random_u32(&kvm_rng) % 9) + 2; + test_tsc_scaling(l0_tsc_freq, l1_scale, l2_scale); + return 0; } -- 2.56.0.rc1.315.gc6ed9934b7-goog Track L2's multiplier, not just the non-fractional scale-up factor, so that the nested TSC scaling test can also validate scaling L2's frequency *down*. No functional change intended. Signed-off-by: Sean Christopherson --- .../kvm/x86/nested_tsc_scaling_test.c | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index d3208b57eafc..af85fd469ef4 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -16,8 +16,8 @@ #include "svm_util.h" #include "kselftest.h" -/* L2 is scaled up (from L1's perspective) by this factor */ -static u64 l2_scale_factor; +/* L2's TSC multiplier, relative to L1. */ +static u64 l2_multiplier; #define TSC_OFFSET_L2 ((u64)-33125236320908) @@ -80,7 +80,7 @@ static void l1_svm_code(struct svm_test_data *svm) generic_svm_setup(svm, l2_guest_code); /* enable TSC scaling for L2 */ - wrmsr(MSR_AMD64_TSC_RATIO, l2_scale_factor << 32); + wrmsr(MSR_AMD64_TSC_RATIO, l2_multiplier); /* launch L2 */ run_guest(svm->vmcb, svm->vmcb_gpa); @@ -115,7 +115,7 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages) vmwrite(SECONDARY_VM_EXEC_CONTROL, control); vmwrite(TSC_OFFSET, TSC_OFFSET_L2); - vmwrite(TSC_MULTIPLIER, l2_scale_factor << 48); + vmwrite(TSC_MULTIPLIER, l2_multiplier); /* launch L2 */ vmlaunch(); @@ -135,20 +135,22 @@ static void l1_guest_code(void *data) l1_svm_code(data); } -static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale) +static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale_factor) { u64 tsc_khz, l1_tsc_freq, l2_tsc_freq; struct kvm_vcpu *vcpu; struct kvm_vm *vm; gva_t guest_gva; - - l2_scale_factor = l2_scale; + u8 frac_bits; printf("L1's scale down factor is: %lu\n", l1_scale_factor); printf("L2's scale up factor is: %lu\n", l2_scale_factor); + frac_bits = kvm_cpu_has(X86_FEATURE_VMX) ? 48 : 32; + l2_multiplier = l2_scale_factor << frac_bits; + vm = vm_create_with_one_vcpu(&vcpu, l1_guest_code); - sync_global_to_guest(vm, l2_scale_factor); + sync_global_to_guest(vm, l2_multiplier); if (kvm_cpu_has(X86_FEATURE_VMX)) vcpu_alloc_vmx(vm, &guest_gva); -- 2.56.0.rc1.315.gc6ed9934b7-goog Print out which one of L0, L1, or L2 TSC frequency checks failed if the nested TSC scaling test detects an out-of-bounds frequency. Triaging test failures without the level information is unnecessarily painful. Signed-off-by: Sean Christopherson --- .../selftests/kvm/x86/nested_tsc_scaling_test.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index af85fd469ef4..8c96b8e501f9 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -32,7 +32,7 @@ enum { USLEEP, UCHECK_L1, UCHECK_L2 }; * measurements, a difference of 1% between the actual and the expected value * is tolerated. */ -static void host_check_tsc_freq(u64 actual, u64 expected) +static void host_check_tsc_freq(int level, u64 actual, u64 expected) { u64 tolerance, thresh_low, thresh_high; @@ -41,8 +41,8 @@ static void host_check_tsc_freq(u64 actual, u64 expected) thresh_high = expected + tolerance; TEST_ASSERT(thresh_low < actual && thresh_high > actual, - "TSC freq is '%lu', expected to be between %lu and %lu", - actual, thresh_low, thresh_high); + "L%u TSC freq is '%lu', expected to be between %lu and %lu", + level, actual, thresh_low, thresh_high); } static void guest_check_tsc_freq(int level) @@ -186,14 +186,14 @@ static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale_ l1_tsc_freq = uc.args[1]; printf("L1's TSC frequency is around: %lu\n", l1_tsc_freq); - host_check_tsc_freq(l1_tsc_freq, + host_check_tsc_freq(1, l1_tsc_freq, l0_tsc_freq / l1_scale_factor); break; case UCHECK_L2: l2_tsc_freq = uc.args[1]; printf("L2's TSC frequency is around: %lu\n", l2_tsc_freq); - host_check_tsc_freq(l2_tsc_freq, + host_check_tsc_freq(2, l2_tsc_freq, l1_tsc_freq * l2_scale_factor); break; } -- 2.56.0.rc1.315.gc6ed9934b7-goog Massage the nested TSC scaling test's checker to allow for validating an expected TSC frequency of 1Hz, which is achievable for L2 since L1 can use a completely arbitrary fractional ratio. To do so, set the minimum tolerance to 1, and make the thresholds inclusive, i.e. for an expected frequency of 1, allow the actual frequency to be in the range of [0,2]. Because scaling L2's frequency down to a single Hz makes the virtual TSC run soooo comically slow, and because all of the math is done with basic integer arithmetic, it's very easy for the test to observe what appears to be a frequency of 0. Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index 8c96b8e501f9..d790ab36a198 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -36,11 +36,11 @@ static void host_check_tsc_freq(int level, u64 actual, u64 expected) { u64 tolerance, thresh_low, thresh_high; - tolerance = expected / 100; + tolerance = max(expected / 100, (u64)1); thresh_low = expected - tolerance; thresh_high = expected + tolerance; - TEST_ASSERT(thresh_low < actual && thresh_high > actual, + TEST_ASSERT(thresh_low <= actual && thresh_high >= actual, "L%u TSC freq is '%lu', expected to be between %lu and %lu", level, actual, thresh_low, thresh_high); } -- 2.56.0.rc1.315.gc6ed9934b7-goog Use KVM's reported default TSC frequency as L0's frequency instead of calibrating L0's frequency using RDTSC. Trying to calibrate the frequency in userspace is all but guaranteed to be wildly inaccurate, e.g. due to interrupts, preemption, etc, and no known CPU supports TSC scaling without also having a constant TSC. I.e. there's no need to calibrate the current frequency, as it should also be the same as KVM's default frequency. Opportunistically sanity check KVM's reported frequency against what is observed via RDTSC, e.g. so that KVM_GET_TSC_KHZ or platform issues show up much earlier in the test, before VMs get involved. Signed-off-by: Sean Christopherson --- .../kvm/x86/nested_tsc_scaling_test.c | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index d790ab36a198..a78585af0ffd 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -137,7 +137,7 @@ static void l1_guest_code(void *data) static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale_factor) { - u64 tsc_khz, l1_tsc_freq, l2_tsc_freq; + u64 l1_tsc_freq, l2_tsc_freq; struct kvm_vcpu *vcpu; struct kvm_vm *vm; gva_t guest_gva; @@ -159,11 +159,8 @@ static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale_ vcpu_args_set(vcpu, 1, guest_gva); - tsc_khz = __vcpu_ioctl(vcpu, KVM_GET_TSC_KHZ, NULL); - TEST_ASSERT(tsc_khz != -1, "vcpu ioctl KVM_GET_TSC_KHZ failed"); - /* scale down L1's TSC frequency */ - vcpu_ioctl(vcpu, KVM_SET_TSC_KHZ, (void *) (tsc_khz / l1_scale_factor)); + vcpu_ioctl(vcpu, KVM_SET_TSC_KHZ, (void *) (l0_tsc_freq / l1_scale_factor)); /* L1 will communicate its frequency before the L2 check.*/ l1_tsc_freq = 0; @@ -212,18 +209,30 @@ static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale_ int main(int argc, char *argv[]) { u64 l0_tsc_freq, tsc_start, tsc_end, l1_scale, l2_scale; + struct kvm_vm *vm; TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_VMX) || kvm_cpu_has(X86_FEATURE_SVM)); TEST_REQUIRE(kvm_has_cap(KVM_CAP_TSC_CONTROL)); TEST_REQUIRE(sys_clocksource_is_based_on_tsc()); + /* + * Create a dummy VM to get KVM's default TSC frequency. All CPUs that + * support TSC scaling should have a constant TSC, i.e. there's no need + * to calibrate the "real" TSC. But do sanity check that the observed + * TSC is within range of KVM's reported TSC frequency. + */ + vm = vm_create_barebones(); + l0_tsc_freq = (u64)__vm_ioctl(vm, KVM_GET_TSC_KHZ, NULL) * 1000; + TEST_ASSERT(l0_tsc_freq, "vcpu ioctl KVM_GET_TSC_KHZ failed"); + kvm_vm_free(vm); + + printf("L0 TSC frequency is: %lu\n", l0_tsc_freq); + tsc_start = rdtsc(); sleep(1); tsc_end = rdtsc(); - - l0_tsc_freq = tsc_end - tsc_start; - printf("real TSC frequency is around: %lu\n", l0_tsc_freq); + host_check_tsc_freq(0, tsc_end - tsc_start, l0_tsc_freq); /* Scale L1 "down" and L2 "up" at a random factor from 2 to 10. */ l1_scale = (kvm_random_u32(&kvm_rng) % 9) + 2; -- 2.56.0.rc1.315.gc6ed9934b7-goog Explicitly pass the L1 and L2 TSC frequencies, along with the L2 multiplier, to the main test routine in the nested TSC scaling test. This will allow testing both "scale up" and "scale down" scenarios, and that KVM saturates L2's frequency if it's scale up/down beyond what hardware can support, in which case the expected frequency will not exactly match the L2 multiplier. Signed-off-by: Sean Christopherson --- .../kvm/x86/nested_tsc_scaling_test.c | 42 ++++++++----------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index a78585af0ffd..a8f9307ed2a4 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -8,7 +8,7 @@ * both L1 and L2 are scaled using different ratios. For this test we scale * L1 down and scale L2 up. */ - +#include #include #include "kvm_util.h" @@ -135,21 +135,19 @@ static void l1_guest_code(void *data) l1_svm_code(data); } -static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale_factor) +static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_tsc_freq, u64 l2_tsc_freq, + u64 __l2_multiplier) { - u64 l1_tsc_freq, l2_tsc_freq; struct kvm_vcpu *vcpu; struct kvm_vm *vm; gva_t guest_gva; - u8 frac_bits; - printf("L1's scale down factor is: %lu\n", l1_scale_factor); - printf("L2's scale up factor is: %lu\n", l2_scale_factor); - - frac_bits = kvm_cpu_has(X86_FEATURE_VMX) ? 48 : 32; - l2_multiplier = l2_scale_factor << frac_bits; + printf("Testing L0 freq = %lu, L1 freq = %lu, L2 freq = %lu, L2 mult = 0x%lx\n", + l0_tsc_freq, l1_tsc_freq, l2_tsc_freq, __l2_multiplier); vm = vm_create_with_one_vcpu(&vcpu, l1_guest_code); + + l2_multiplier = __l2_multiplier; sync_global_to_guest(vm, l2_multiplier); if (kvm_cpu_has(X86_FEATURE_VMX)) @@ -159,11 +157,7 @@ static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale_ vcpu_args_set(vcpu, 1, guest_gva); - /* scale down L1's TSC frequency */ - vcpu_ioctl(vcpu, KVM_SET_TSC_KHZ, (void *) (l0_tsc_freq / l1_scale_factor)); - - /* L1 will communicate its frequency before the L2 check.*/ - l1_tsc_freq = 0; + vcpu_ioctl(vcpu, KVM_SET_TSC_KHZ, (void *)(l1_tsc_freq / 1000)); for (;;) { struct ucall uc; @@ -180,18 +174,12 @@ static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale_ sleep(uc.args[1]); break; case UCHECK_L1: - l1_tsc_freq = uc.args[1]; - printf("L1's TSC frequency is around: %lu\n", l1_tsc_freq); - - host_check_tsc_freq(1, l1_tsc_freq, - l0_tsc_freq / l1_scale_factor); + printf("L1's observed TSC frequency: %lu\n", uc.args[1]); + host_check_tsc_freq(1, uc.args[1], l1_tsc_freq); break; case UCHECK_L2: - l2_tsc_freq = uc.args[1]; - printf("L2's TSC frequency is around: %lu\n", l2_tsc_freq); - - host_check_tsc_freq(2, l2_tsc_freq, - l1_tsc_freq * l2_scale_factor); + printf("L2's observed TSC frequency: %lu\n", uc.args[1]); + host_check_tsc_freq(2, uc.args[1], l2_tsc_freq); break; } break; @@ -209,6 +197,7 @@ static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_scale_factor, u64 l2_scale_ int main(int argc, char *argv[]) { u64 l0_tsc_freq, tsc_start, tsc_end, l1_scale, l2_scale; + u8 frac_bits = kvm_cpu_has(X86_FEATURE_VMX) ? 48 : 32; struct kvm_vm *vm; TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_VMX) || @@ -237,7 +226,10 @@ int main(int argc, char *argv[]) /* Scale L1 "down" and L2 "up" at a random factor from 2 to 10. */ l1_scale = (kvm_random_u32(&kvm_rng) % 9) + 2; l2_scale = (kvm_random_u32(&kvm_rng) % 9) + 2; - test_tsc_scaling(l0_tsc_freq, l1_scale, l2_scale); + + test_tsc_scaling(l0_tsc_freq, l0_tsc_freq / l1_scale, + mul_u64_u64_div64(l0_tsc_freq, l2_scale, l1_scale), + (l2_scale << frac_bits)); return 0; } -- 2.56.0.rc1.315.gc6ed9934b7-goog Explicitly verify that both L1 and L2 can run at KVM's default TSC frequency reported via KVM_GET_TSC_KHZ, e.g. that KVM_SET_TSC_KHZ can take in what KVM_GET_TSC_KHZ spits out. Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index a8f9307ed2a4..ee8fda310e33 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -223,6 +223,9 @@ int main(int argc, char *argv[]) tsc_end = rdtsc(); host_check_tsc_freq(0, tsc_end - tsc_start, l0_tsc_freq); + /* Sanity check the frequency reported by KVM_GET_TSC_KHZ. */ + test_tsc_scaling(l0_tsc_freq, l0_tsc_freq, l0_tsc_freq, BIT_ULL(frac_bits)); + /* Scale L1 "down" and L2 "up" at a random factor from 2 to 10. */ l1_scale = (kvm_random_u32(&kvm_rng) % 9) + 2; l2_scale = (kvm_random_u32(&kvm_rng) % 9) + 2; -- 2.56.0.rc1.315.gc6ed9934b7-goog Now that the guts of the nested TSC scaling test support scaling L2 down, validate all four combinations of scaling up/down L1 and L2. Ideally, the test would also validate integer+fractional scale up scenarios, but prioritize keeping the test and math somewhat understandable. Opportunistically drop the stale file comment about the test's name. Signed-off-by: Sean Christopherson --- .../kvm/x86/nested_tsc_scaling_test.c | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index ee8fda310e33..1d578af393bd 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -1,12 +1,9 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * vmx_nested_tsc_scaling_test - * * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. * * This test case verifies that nested TSC scaling behaves as expected when - * both L1 and L2 are scaled using different ratios. For this test we scale - * L1 down and scale L2 up. + * both L1 and L2 are scaled using different ratios. */ #include #include @@ -226,7 +223,21 @@ int main(int argc, char *argv[]) /* Sanity check the frequency reported by KVM_GET_TSC_KHZ. */ test_tsc_scaling(l0_tsc_freq, l0_tsc_freq, l0_tsc_freq, BIT_ULL(frac_bits)); - /* Scale L1 "down" and L2 "up" at a random factor from 2 to 10. */ + /* + * Scale L1 and L2 up and down at random factors from 2 to 10. Current + * CPUs have two full orders of magnitude of "breathing room" before an + * ultrafast L0 TSC frequency multiplied by 10x will encounter KVM's + * signed 32-bit limit on L1's frequency, and it's highly unlikely a + * CPU that supports TSC scaling will show up running at 100MHz, i.e. + * underflowing KVM's minimum 1KHz frequency is extremely unlikely. + * + * For L2, the frequency is limited only by what hardware can support, + * not by KVM's limits. SVM provides 8 bits of integer scale up, and + * 32 bits of fractional scale down, i.e. can scale up 255x and down a + * comical amount, so scaling up 100x and down 1/100 is well within + * hardware's capabilities (VMX provides 16 bits of "up" and 48 bits of + * "down"). + */ l1_scale = (kvm_random_u32(&kvm_rng) % 9) + 2; l2_scale = (kvm_random_u32(&kvm_rng) % 9) + 2; @@ -234,5 +245,17 @@ int main(int argc, char *argv[]) mul_u64_u64_div64(l0_tsc_freq, l2_scale, l1_scale), (l2_scale << frac_bits)); + test_tsc_scaling(l0_tsc_freq, l0_tsc_freq * l1_scale, + mul_u64_u64_div64(l0_tsc_freq, l1_scale, l2_scale), + (1ull << frac_bits) / l2_scale); + + test_tsc_scaling(l0_tsc_freq, l0_tsc_freq / l1_scale, + l0_tsc_freq / l1_scale / l2_scale, + (1ull << frac_bits) / l2_scale); + + test_tsc_scaling(l0_tsc_freq, l0_tsc_freq * l1_scale, + l0_tsc_freq * l1_scale * l2_scale, + (l2_scale << frac_bits)); + return 0; } -- 2.56.0.rc1.315.gc6ed9934b7-goog Extend the nested TSC scaling test to validate the extreme ends of what hardware supports, i.e. validate that KVM allows L1 to scale L2's virtual TSC frequency up/down to the architectural maximum/minimum, from L1's perspective, and that KVM saturates L2's effective frequency when L2's frequency can't be virtualized by hardware (which is possible when stacking multipliers/ratios across L1 and L2). Signed-off-by: Sean Christopherson --- .../kvm/x86/nested_tsc_scaling_test.c | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index 1d578af393bd..4cacd87f129b 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -193,6 +193,7 @@ static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_tsc_freq, u64 l2_tsc_freq, int main(int argc, char *argv[]) { + u64 min_freq, min_multiplier, max_freq, max_multiplier, l1_max_freq, l1_min_freq; u64 l0_tsc_freq, tsc_start, tsc_end, l1_scale, l2_scale; u8 frac_bits = kvm_cpu_has(X86_FEATURE_VMX) ? 48 : 32; struct kvm_vm *vm; @@ -257,5 +258,39 @@ int main(int argc, char *argv[]) l0_tsc_freq * l1_scale * l2_scale, (l2_scale << frac_bits)); + /* + * Test that KVM saturates L2's frequency on both ends if the resulting + * L2 TSC frequency would be below or above what hardware can support. + * Because L2 = L0 * (L1_mult >> frac) * (L2_mult >> frac) needs to be + * distilled down to a single multiplier, very small/large multipliers + * will underflow/overflow the minimum/maximum multiplier supported by + * hardware when L1 and L2 multipliers are combined. KVM's behavior is + * saturate on {under,over}flow, i.e. to run at the min/max frequency. + * + * Note, userspace can only program L1's frequency in KHz, i.e. can't + * specify an exact multiplier. As a result, the minimum and maximum + * frequencies are different for L1 vs L2, because L1 is constrained by + * hardware *and* KVM, whereas L2 is constrained only by hardware. + */ + min_multiplier = 1; + min_freq = mul_u64_u64_div64(l0_tsc_freq, min_multiplier, BIT_ULL(frac_bits)); + min_freq = max(min_freq, (u64)1); + l1_min_freq = max(min_freq, (u64)1 * 1000); + test_tsc_scaling(l0_tsc_freq, l1_min_freq, min_freq, 1); + + /* + * SVM takes a 40-bit value (right shifted by 32), while VMX takes a + * 64-bit value (right shifted by 48). mul_u64_u64_shr() isn't (yet) + * available in selftests, but mul_u64_u64_div64() does nicely since, + * albeit more slowly (performance is obviously not a concern). Note, + * because KVM_GET_TSC_KHZ returns a signed 32-bit integer, KVM limits + * KVM_SET_TSC_KHZ to INT_MAX, even though hardware (both SVM and VMX) + * supports much higher frequencies. + */ + max_multiplier = kvm_cpu_has(X86_FEATURE_VMX) ? -1ull : GENMASK_U64(39, 0); + max_freq = mul_u64_u64_div64(l0_tsc_freq, max_multiplier, BIT_ULL(frac_bits)); + l1_max_freq = min(max_freq, (u64)INT32_MAX * 1000); + test_tsc_scaling(l0_tsc_freq, l1_max_freq, max_freq, max_multiplier); + return 0; } -- 2.56.0.rc1.315.gc6ed9934b7-goog Set L2's TSC offset for SVM as well as VMX in the nested TSC scaling test. Odds are good skipping SVM was pure oversight. Fixes: e6bcdd212238 ("KVM: selftests: Extend vmx_nested_tsc_scaling_test to cover SVM") Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index 4cacd87f129b..571fc214c925 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -75,6 +75,7 @@ static void l1_svm_code(struct svm_test_data *svm) guest_check_tsc_freq(UCHECK_L1); generic_svm_setup(svm, l2_guest_code); + svm->vmcb->control.tsc_offset = TSC_OFFSET_L2; /* enable TSC scaling for L2 */ wrmsr(MSR_AMD64_TSC_RATIO, l2_multiplier); -- 2.56.0.rc1.315.gc6ed9934b7-goog Randomize L2's TSC offset in the nested TSC scaling test, not because it's all that interesting, but because hardcoding a copmletely arbitrary value is all kinds of silly. Signed-off-by: Sean Christopherson --- .../selftests/kvm/x86/nested_tsc_scaling_test.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index 571fc214c925..be6246c20d7b 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -13,10 +13,9 @@ #include "svm_util.h" #include "kselftest.h" -/* L2's TSC multiplier, relative to L1. */ +/* L2's TSC multiplier and offset, relative to L1. */ static u64 l2_multiplier; - -#define TSC_OFFSET_L2 ((u64)-33125236320908) +static u64 l2_offset; enum { USLEEP, UCHECK_L1, UCHECK_L2 }; #define GUEST_SLEEP(sec) ucall(UCALL_SYNC, 2, USLEEP, sec) @@ -75,7 +74,7 @@ static void l1_svm_code(struct svm_test_data *svm) guest_check_tsc_freq(UCHECK_L1); generic_svm_setup(svm, l2_guest_code); - svm->vmcb->control.tsc_offset = TSC_OFFSET_L2; + svm->vmcb->control.tsc_offset = l2_offset; /* enable TSC scaling for L2 */ wrmsr(MSR_AMD64_TSC_RATIO, l2_multiplier); @@ -112,7 +111,7 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages) control |= SECONDARY_EXEC_TSC_SCALING; vmwrite(SECONDARY_VM_EXEC_CONTROL, control); - vmwrite(TSC_OFFSET, TSC_OFFSET_L2); + vmwrite(TSC_OFFSET, l2_offset); vmwrite(TSC_MULTIPLIER, l2_multiplier); /* launch L2 */ @@ -145,6 +144,9 @@ static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_tsc_freq, u64 l2_tsc_freq, vm = vm_create_with_one_vcpu(&vcpu, l1_guest_code); + l2_offset = kvm_random_u64(&kvm_rng); + sync_global_to_guest(vm, l2_offset); + l2_multiplier = __l2_multiplier; sync_global_to_guest(vm, l2_multiplier); -- 2.56.0.rc1.315.gc6ed9934b7-goog Use GUEST_SYNC2() instead of open coding equivalent functionality using raw ucall() invocations in the nested TSC scaling test. No functional change intended. Signed-off-by: Sean Christopherson --- tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index be6246c20d7b..b21f79e9ec71 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -18,9 +18,6 @@ static u64 l2_multiplier; static u64 l2_offset; enum { USLEEP, UCHECK_L1, UCHECK_L2 }; -#define GUEST_SLEEP(sec) ucall(UCALL_SYNC, 2, USLEEP, sec) -#define GUEST_CHECK(level, freq) ucall(UCALL_SYNC, 2, level, freq) - /* * This function checks whether the "actual" TSC frequency of a guest matches @@ -52,12 +49,12 @@ static void guest_check_tsc_freq(int level) * be good enough for the purposes of this test. */ tsc_start = rdmsr(MSR_IA32_TSC); - GUEST_SLEEP(1); + GUEST_SYNC2(USLEEP, 1); tsc_end = rdmsr(MSR_IA32_TSC); tsc_freq = tsc_end - tsc_start; - GUEST_CHECK(level, tsc_freq); + GUEST_SYNC2(level, tsc_freq); } static void l2_guest_code(void) -- 2.56.0.rc1.315.gc6ed9934b7-goog Explicitly spell out UCALL instead of prefixing only U in the nested TSC scaling test's ucall commands, as it's all too easy to think USLEEP means "do usleep()", when it actually means "do sleep() in userspace". Opportunistically name the enum and take an enum instead of a bare integer for the to-be-checked level, to make it more obvious that the fact that the level values happen to correspond to '1' and '2' for the nesting level is largely coincidental (or perhaps it was intentional, but it's not a hard requirement for the test). Signed-off-by: Sean Christopherson --- .../kvm/x86/nested_tsc_scaling_test.c | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c index b21f79e9ec71..bfb1ff441c78 100644 --- a/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c +++ b/tools/testing/selftests/kvm/x86/nested_tsc_scaling_test.c @@ -17,7 +17,7 @@ static u64 l2_multiplier; static u64 l2_offset; -enum { USLEEP, UCHECK_L1, UCHECK_L2 }; +enum ucall_cmd { UCALL_SLEEP, UCALL_CHECK_L1, UCALL_CHECK_L2 }; /* * This function checks whether the "actual" TSC frequency of a guest matches @@ -38,7 +38,7 @@ static void host_check_tsc_freq(int level, u64 actual, u64 expected) level, actual, thresh_low, thresh_high); } -static void guest_check_tsc_freq(int level) +static void guest_check_tsc_freq(enum ucall_cmd check_level) { u64 tsc_start, tsc_end, tsc_freq; @@ -49,17 +49,17 @@ static void guest_check_tsc_freq(int level) * be good enough for the purposes of this test. */ tsc_start = rdmsr(MSR_IA32_TSC); - GUEST_SYNC2(USLEEP, 1); + GUEST_SYNC2(UCALL_SLEEP, 1); tsc_end = rdmsr(MSR_IA32_TSC); tsc_freq = tsc_end - tsc_start; - GUEST_SYNC2(level, tsc_freq); + GUEST_SYNC2(check_level, tsc_freq); } static void l2_guest_code(void) { - guest_check_tsc_freq(UCHECK_L2); + guest_check_tsc_freq(UCALL_CHECK_L2); /* exit to L1 */ __asm__ __volatile__("vmcall"); @@ -68,7 +68,7 @@ static void l2_guest_code(void) static void l1_svm_code(struct svm_test_data *svm) { /* check that L1's frequency looks alright before launching L2 */ - guest_check_tsc_freq(UCHECK_L1); + guest_check_tsc_freq(UCALL_CHECK_L1); generic_svm_setup(svm, l2_guest_code); svm->vmcb->control.tsc_offset = l2_offset; @@ -81,7 +81,7 @@ static void l1_svm_code(struct svm_test_data *svm) GUEST_ASSERT(svm->vmcb->control.exit_code == SVM_EXIT_VMMCALL); /* check that L1's frequency still looks good */ - guest_check_tsc_freq(UCHECK_L1); + guest_check_tsc_freq(UCALL_CHECK_L1); GUEST_DONE(); } @@ -91,7 +91,7 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages) u32 control; /* check that L1's frequency looks alright before launching L2 */ - guest_check_tsc_freq(UCHECK_L1); + guest_check_tsc_freq(UCALL_CHECK_L1); prepare_for_vmx_operation(vmx_pages); load_vmcs(vmx_pages); @@ -116,7 +116,7 @@ static void l1_vmx_code(struct vmx_pages *vmx_pages) GUEST_ASSERT(vmread(VM_EXIT_REASON) == EXIT_REASON_VMCALL); /* check that L1's frequency still looks good */ - guest_check_tsc_freq(UCHECK_L1); + guest_check_tsc_freq(UCALL_CHECK_L1); GUEST_DONE(); } @@ -167,14 +167,14 @@ static void test_tsc_scaling(u64 l0_tsc_freq, u64 l1_tsc_freq, u64 l2_tsc_freq, REPORT_GUEST_ASSERT(uc); case UCALL_SYNC: switch (uc.args[0]) { - case USLEEP: + case UCALL_SLEEP: sleep(uc.args[1]); break; - case UCHECK_L1: + case UCALL_CHECK_L1: printf("L1's observed TSC frequency: %lu\n", uc.args[1]); host_check_tsc_freq(1, uc.args[1], l1_tsc_freq); break; - case UCHECK_L2: + case UCALL_CHECK_L2: printf("L2's observed TSC frequency: %lu\n", uc.args[1]); host_check_tsc_freq(2, uc.args[1], l2_tsc_freq); break; -- 2.56.0.rc1.315.gc6ed9934b7-goog