Thanks for looking at this again. I'm glad to know there is still
interest in upstreaming it. It fell off my radar and other things took
priority.
Sean Christopherson <seanjc@xxxxxxxxxx> writes:
On Mon, Mar 27, 2023, Colton Lewis wrote:
+uint64_t cycles_read(void);
I would prefer something like get_system_counter() or
read_system_counter()
Pairing "read" after "cycles" can be read (lol) in past tense or current
tense,
e.g. "the number of cycles that were read" versus "read the current
number of
cycles". I used guest_system_counter_read() in an example in v1[*], but
that was
just me copy+pasting from the patch.
And "cycles" is typically used to describe latency and elapsed time, e.g.
doing
uint64_t time = cycles_to_ns(cycles_read());
looks valid at a glance, e.g. "convert that number of cycles that were
read into
nanoseconds", but is nonsensical in most cases because it's current
tense, and
there's no baseline time.
Sorry for not bringing this up in v2, I think I only looked at the
implementation.
[*] https://lore.kernel.org/kvm/Y9LPhs1BgBA4+kBY@xxxxxxxxxx
I'll change to get_system_counter_ordered() as Oliver suggests.
+uint64_t cycles_to_ns(struct kvm_vcpu *vcpu, uint64_t cycles)
+{
+ TEST_ASSERT(cycles < 10000000000, "Conversion to ns may overflow");
+ return cycles * NSEC_PER_SEC / timer_get_cntfrq();
+}
diff --git a/tools/testing/selftests/kvm/lib/x86_64/processor.c
b/tools/testing/selftests/kvm/lib/x86_64/processor.c
index ae1e573d94ce..adef76bebff3 100644
--- a/tools/testing/selftests/kvm/lib/x86_64/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86_64/processor.c
@@ -1270,3 +1270,16 @@ void kvm_selftest_arch_init(void)
host_cpu_is_intel = this_cpu_is_intel();
host_cpu_is_amd = this_cpu_is_amd();
}
+
+uint64_t cycles_read(void)
+{
+ return rdtsc();
+}
+
+uint64_t cycles_to_ns(struct kvm_vcpu *vcpu, uint64_t cycles)
+{
+ uint64_t tsc_khz = __vcpu_ioctl(vcpu, KVM_GET_TSC_KHZ, NULL);
+
+ TEST_ASSERT(cycles < 10000000000, "Conversion to ns may overflow");
Is it possible to calculate this programatically instead of hardcoding a
magic
number?
It is. Unsure why I didn't do it before.
