Re: [PATCH V9 1/2] KVM: selftests: Add x86_64 guest udelay() utility

Sean Christopherson
2024-06-28T22:46:59Z

On Wed, Jun 12, 2024, Reinette Chatre wrote:
> ---
> .../selftests/kvm/include/x86_64/processor.h | 17 +++++++++++++++++
> .../selftests/kvm/lib/x86_64/processor.c | 11 +++++++++++
> 2 files changed, 28 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/include/x86_64/processor.h b/tools/testing/selftests/kvm/include/x86_64/processor.h
> index c0c7c1fe93f9..383a0f7fa9ef 100644
> --- a/tools/testing/selftests/kvm/include/x86_64/processor.h
> +++ b/tools/testing/selftests/kvm/include/x86_64/processor.h
> @@ -23,6 +23,7 @@
>
> extern bool host_cpu_is_intel;
> extern bool host_cpu_is_amd;
> +extern uint32_t tsc_khz;

This should be guest_tsc_khz, because it most definitely isn't guaranteed to be
the host TSC frequency. And because it's global, we should try to avoid variable
shadowing, e.g. tsc_scaling_sync.c also defines tsc_khz.

Which, by the by, probably needs to be addressed, i.e. we should probably add a
helper for setting KVM_SET_TSC_KHZ+guest_tsc_khz.

I think it also makes sense to have this be a 64-bit value, even though KVM
*internally* tracks a 32-bit value. That way we don't have to worry about
casting to avoid truncation.

> /* Forced emulation prefix, used to invoke the emulator unconditionally. */
> #define KVM_FEP "ud2; .byte 'k', 'v', 'm';"
> @@ -816,6 +817,22 @@ static inline void cpu_relax(void)
> asm volatile("rep; nop" ::: "memory");
> }
>
> +static inline void udelay(unsigned long usec)
> +{
> + uint64_t start, now, cycles;
> +
> + GUEST_ASSERT(tsc_khz);
> + cycles = tsc_khz / 1000 * usec;
> +
> + start = rdtsc();
> + for (;;) {
> + now = rdtsc();
> + if (now - start >= cycles)
> + break;
> + cpu_relax();

Given that this is guest code, we should omit the PAUSE so that it doesn't trigger
PLE exits, i.e. to make the delay as accurate as possible. Then this simply becomes:

start = rdtsc();
do {
now = rdtsc();
} while (now - start < cycles);