linux-kernel - Re: [PATCH V9 1/2] KVM: selftests: Add x86

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Zn89YTYQcEEu9Jrw@google.com>
Date: Fri, 28 Jun 2024 15:46:57 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Reinette Chatre <reinette.chatre@...el.com>
Cc: isaku.yamahata@...el.com, pbonzini@...hat.com, erdemaktas@...gle.com, 
	vkuznets@...hat.com, vannapurve@...gle.com, jmattson@...gle.com, 
	mlevitsk@...hat.com, xiaoyao.li@...el.com, chao.gao@...el.com, 
	rick.p.edgecombe@...el.com, yuan.yao@...el.com, kvm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V9 1/2] KVM: selftests: Add x86_64 guest udelay() utility

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);